Skip to content

Commit 302ccc5

Browse files
chrapkowski-sgchrapkowski-sg
committed
fix/improve-error-messages
1 parent f404049 commit 302ccc5

File tree

2 files changed

+112
-75
lines changed

2 files changed

+112
-75
lines changed

‎cmd/src/code_intel_upload.go‎

Lines changed: 94 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func handleCodeIntelUpload(args []string) error {
7171
}
7272
}
7373
if err != nil {
74-
return handleUploadError(cfg.AccessToken, err)
74+
return err
7575
}
7676

7777
client := cfg.apiClient(codeintelUploadFlags.apiFlags, io.Discard)
@@ -212,94 +212,123 @@ func (e errorWithHint) Error() string {
212212
return fmt.Sprintf("%s\n\n%s\n", e.err, e.hint)
213213
}
214214

215-
// handleUploadError writes the given error to the given output. If the
216-
// given output object is nil then the error will be written to standard out.
217-
//
218-
// This method returns the error that should be passed back up to the runner.
215+
// handleUploadError attaches actionable hints to upload errors and returns
216+
// the enriched error. Only called for actual upload failures (not flag validation).
219217
func handleUploadError(accessToken string, err error) error {
220-
if errors.Is(err, upload.ErrUnauthorized) {
221-
err = attachHintsForAuthorizationError(accessToken, err)
218+
var httpErr *ErrUnexpectedStatusCode
219+
if errors.As(err, &httpErr) && (httpErr.Code == 401 || httpErr.Code == 403) {
220+
isUnauthorized := httpErr.Code == 401
221+
isForbidden := httpErr.Code == 403
222+
223+
displayErr := errors.Newf("upload failed: %s", uploadFailureReason(httpErr))
224+
225+
err = errorWithHint{
226+
err: displayErr,
227+
hint: uploadHints(accessToken, isUnauthorized, isForbidden),
228+
}
222229
}
223230

224231
if codeintelUploadFlags.ignoreUploadFailures {
225-
// Report but don't return the error
226232
fmt.Println(err.Error())
227233
return nil
228234
}
229235

230236
return err
231237
}
232238

233-
func attachHintsForAuthorizationError(accessToken string, originalError error) error {
234-
var actionableHints []string
239+
// uploadHints builds hint paragraphs for the Sourcegraph access token,
240+
// code host tokens, and a docs link.
241+
func uploadHints(accessToken string, isUnauthorized, isForbidden bool) string {
242+
var hints []string
235243

236-
likelyTokenError := accessToken == ""
237-
if _, parseErr := accesstoken.ParsePersonalAccessToken(accessToken); accessToken != "" && parseErr != nil {
238-
likelyTokenError = true
239-
actionableHints = append(actionableHints,
240-
"However, the provided access token does not match expected format; was it truncated?",
241-
"Typically the access token looks like sgp_<40 hex chars> or sgp_<instance-id>_<40 hex chars>.")
244+
if h := sourcegraphAccessTokenHint(accessToken, isUnauthorized, isForbidden); h != "" {
245+
hints = append(hints, h)
242246
}
243247

244-
if likelyTokenError {
245-
return errorWithHint{err: originalError, hint: strings.Join(mergeStringSlices(
246-
[]string{"A Sourcegraph access token must be provided via SRC_ACCESS_TOKEN for uploading SCIP data."},
247-
actionableHints,
248-
[]string{"For more details, see https://sourcegraph.com/docs/cli/how-tos/creating_an_access_token."},
249-
), "\n")}
250-
}
248+
hints = append(hints, codeHostTokenHints(isUnauthorized)...)
251249

252-
needsGitHubToken := strings.HasPrefix(codeintelUploadFlags.repo, "github.com")
253-
needsGitLabToken := strings.HasPrefix(codeintelUploadFlags.repo, "gitlab.com")
250+
hints = append(hints, "For more details on uploading SCIP indexes, see https://sourcegraph.com/docs/cli/references/code-intel/upload.")
254251

255-
if needsGitHubToken {
256-
if codeintelUploadFlags.gitHubToken != "" {
257-
actionableHints = append(actionableHints,
258-
fmt.Sprintf("The supplied -github-token does not indicate that you have collaborator access to %s.", codeintelUploadFlags.repo),
259-
"Please check the value of the supplied token and its permissions on the code host and try again.",
260-
)
261-
} else {
262-
actionableHints = append(actionableHints,
263-
fmt.Sprintf("Please retry your request with a -github-token=XXX with collaborator access to %s.", codeintelUploadFlags.repo),
264-
"This token will be used to check with the code host that the uploading user has write access to the target repository.",
265-
)
252+
return strings.Join(hints, "\n\n")
253+
}
254+
255+
// sourcegraphAccessTokenHint returns a hint about the Sourcegraph access token
256+
// based on the error type and token state.
257+
func sourcegraphAccessTokenHint(accessToken string, isUnauthorized, isForbidden bool) string {
258+
if isUnauthorized {
259+
if accessToken == "" {
260+
return "No Sourcegraph access token was provided. Set the SRC_ACCESS_TOKEN environment variable to a valid token."
266261
}
267-
} else if needsGitLabToken {
268-
if codeintelUploadFlags.gitLabToken != "" {
269-
actionableHints = append(actionableHints,
270-
fmt.Sprintf("The supplied -gitlab-token does not indicate that you have write access to %s.", codeintelUploadFlags.repo),
271-
"Please check the value of the supplied token and its permissions on the code host and try again.",
272-
)
273-
} else {
274-
actionableHints = append(actionableHints,
275-
fmt.Sprintf("Please retry your request with a -gitlab-token=XXX with write access to %s.", codeintelUploadFlags.repo),
276-
"This token will be used to check with the code host that the uploading user has write access to the target repository.",
277-
)
262+
if _, parseErr := accesstoken.ParsePersonalAccessToken(accessToken); parseErr != nil {
263+
return "The provided Sourcegraph access token does not match the expected format (sgp_<40 hex chars> or sgp_<instance-id>_<40 hex chars>). Was it copied incorrectly or truncated?"
278264
}
279-
} else {
280-
actionableHints = append(actionableHints,
281-
"Verification is supported for the following code hosts: github.com, gitlab.com.",
282-
"Please request support for additional code host verification at https://github.com/sourcegraph/sourcegraph/issues/4967.",
283-
)
265+
return "The Sourcegraph access token may be invalid, expired, or you may be connecting to the wrong Sourcegraph instance."
266+
}
267+
if isForbidden {
268+
return "You may not have sufficient permissions on this Sourcegraph instance."
269+
}
270+
return ""
271+
}
272+
273+
// codeHostTokenHints returns hints about GitHub and GitLab tokens.
274+
func codeHostTokenHints(isUnauthorized bool) []string {
275+
var hints []string
276+
277+
if h := gitHubTokenHint(isUnauthorized); h != "" {
278+
hints = append(hints, h)
279+
}
280+
if h := gitLabTokenHint(isUnauthorized); h != "" {
281+
hints = append(hints, h)
282+
}
283+
284+
if len(hints) == 0 {
285+
hints = append(hints, "Code host verification is supported for github.com and gitlab.com repositories.")
284286
}
285287

286-
return errorWithHint{err: originalError, hint: strings.Join(mergeStringSlices(
287-
[]string{"This Sourcegraph instance has enforced auth for SCIP uploads."},
288-
actionableHints,
289-
[]string{"For more details, see https://docs.sourcegraph.com/cli/references/code-intel/upload."},
290-
), "\n")}
288+
return hints
291289
}
292290

293-
// emergencyOutput creates a default Output object writing to standard out.
294-
func emergencyOutput() *output.Output {
295-
return output.NewOutput(os.Stdout, output.OutputOpts{})
291+
// gitHubTokenHint returns a hint about the GitHub token, or empty if not applicable.
292+
func gitHubTokenHint(isUnauthorized bool) string {
293+
if codeintelUploadFlags.gitHubToken != "" {
294+
if isUnauthorized {
295+
return "The supplied -github-token may be invalid."
296+
}
297+
return "The supplied -github-token may lack the required permissions."
298+
}
299+
if strings.HasPrefix(codeintelUploadFlags.repo, "github.com") {
300+
return fmt.Sprintf("No -github-token was provided. If this Sourcegraph instance enforces code host authentication, retry with -github-token=<token> for a token with access to %s.", codeintelUploadFlags.repo)
301+
}
302+
return ""
296303
}
297304

298-
func mergeStringSlices(ss ...[]string) []string {
299-
var combined []string
300-
for _, s := range ss {
301-
combined = append(combined, s...)
305+
// gitLabTokenHint returns a hint about the GitLab token, or empty if not applicable.
306+
func gitLabTokenHint(isUnauthorized bool) string {
307+
if codeintelUploadFlags.gitLabToken != "" {
308+
if isUnauthorized {
309+
return "The supplied -gitlab-token may be invalid."
310+
}
311+
return "The supplied -gitlab-token may lack the required permissions."
312+
}
313+
if strings.HasPrefix(codeintelUploadFlags.repo, "gitlab.com") {
314+
return fmt.Sprintf("No -gitlab-token was provided. If this Sourcegraph instance enforces code host authentication, retry with -gitlab-token=<token> for a token with access to %s.", codeintelUploadFlags.repo)
302315
}
316+
return ""
317+
}
318+
319+
// uploadFailureReason returns the server's response body if available, or a
320+
// generic reason derived from the HTTP status code.
321+
func uploadFailureReason(httpErr *ErrUnexpectedStatusCode) string {
322+
if httpErr.Body != "" {
323+
return httpErr.Body
324+
}
325+
if httpErr.Code == 401 {
326+
return "unauthorized"
327+
}
328+
return "forbidden"
329+
}
303330

304-
return combined
331+
// emergencyOutput creates a default Output object writing to standard out.
332+
func emergencyOutput() *output.Output {
333+
return output.NewOutput(os.Stdout, output.OutputOpts{})
305334
}

‎cmd/src/code_intel_upload_vendored.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,20 @@ type uploadRequestOptions struct {
339339
Done bool // Whether the request is a multipart finalize
340340
}
341341

342-
// ErrUnauthorized occurs when the upload endpoint returns a 401 response.
343-
var ErrUnauthorized = errors.New("unauthorized upload")
342+
// ErrUnexpectedStatusCode is returned for HTTP error responses from the upload
343+
// endpoint. It carries the status code and response body so callers can inspect
344+
// them without parsing the error string.
345+
type ErrUnexpectedStatusCode struct {
346+
Code int
347+
Body string
348+
}
349+
350+
func (e *ErrUnexpectedStatusCode) Error() string {
351+
if e.Body != "" {
352+
return fmt.Sprintf("unexpected status code: %d (%s)", e.Code, e.Body)
353+
}
354+
return fmt.Sprintf("unexpected status code: %d", e.Code)
355+
}
344356

345357
// performUploadRequest performs an HTTP POST to the upload endpoint. The query string of the request
346358
// is constructed from the given request options and the body of the request is the unmodified reader.
@@ -419,17 +431,13 @@ func performRequest(ctx context.Context, req *http.Request, httpClient upload.Cl
419431
// returns a boolean flag indicating if the function can be retried on failure (error-dependent).
420432
func decodeUploadPayload(resp *http.Response, body []byte, target *int) (bool, error) {
421433
if resp.StatusCode >= 300 {
422-
if resp.StatusCode == http.StatusUnauthorized {
423-
return false, ErrUnauthorized
424-
}
425-
426-
suffix := ""
427-
if !bytes.HasPrefix(bytes.TrimSpace(body), []byte{'<'}) {
428-
suffix = fmt.Sprintf(" (%s)", bytes.TrimSpace(body))
434+
detail := ""
435+
if trimmed := bytes.TrimSpace(body); len(trimmed) > 0 && trimmed[0] != '<' {
436+
detail = string(trimmed)
429437
}
430438

431439
// Do not retry client errors
432-
return resp.StatusCode >= 500, errors.Errorf("unexpected status code: %d%s", resp.StatusCode, suffix)
440+
return resp.StatusCode >= 500, &ErrUnexpectedStatusCode{Code: resp.StatusCode, Body: detail}
433441
}
434442

435443
if target == nil {

0 commit comments

Comments
 (0)