Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 105 additions & 64 deletions cmd/src/code_intel_upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func handleCodeIntelUpload(args []string) error {
}
}
if err != nil {
return handleUploadError(cfg.AccessToken, err)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed here handleUploadError at this point only flags validation errors may occur and handleUploadError is about handling errors of during actual upload

return err
}

client := cfg.apiClient(codeintelUploadFlags.apiFlags, io.Discard)
Expand Down Expand Up @@ -212,94 +212,135 @@ func (e errorWithHint) Error() string {
return fmt.Sprintf("%s\n\n%s\n", e.err, e.hint)
}

// handleUploadError writes the given error to the given output. If the
// given output object is nil then the error will be written to standard out.
//
// This method returns the error that should be passed back up to the runner.
// handleUploadError attaches actionable hints to upload errors and returns
// the enriched error. Only called for actual upload failures (not flag validation).
func handleUploadError(accessToken string, err error) error {
if errors.Is(err, upload.ErrUnauthorized) {
err = attachHintsForAuthorizationError(accessToken, err)
if httpErr := findAuthError(err); httpErr != nil {
isUnauthorized := httpErr.Code == 401
isForbidden := httpErr.Code == 403

displayErr := errors.Newf("upload failed: %s", uploadFailureReason(httpErr))

err = errorWithHint{
err: displayErr,
hint: uploadHints(accessToken, isUnauthorized, isForbidden),
}
}

if codeintelUploadFlags.ignoreUploadFailures {
// Report but don't return the error
fmt.Println(err.Error())
return nil
}

return err
}

func attachHintsForAuthorizationError(accessToken string, originalError error) error {
var actionableHints []string
// findAuthError searches the error chain (including multi-errors from retries)
// for a 401 or 403 ErrUnexpectedStatusCode. Returns nil if none is found.
Comment on lines +238 to +239
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just checking my understanding: so the reason you can't just use errors.As is it will return the first found httpErr but you want to check all of them to find one which could be a 401/403.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. To give more context.

When retries happen, errors get combined into a MultiError. If e.g. the first attempt gets a (502 which did happen to me sometimes during testing) and a retry gets a 401, errors.As returns the first ErrUnexpectedStatusCode it finds (the 502), so the httpErr.Code == 401 || httpErr.Code == 403 check fails β€” even though an auth error exists in the chain.

func findAuthError(err error) *ErrUnexpectedStatusCode {
// Check if it's a multi-error and scan all children.
if multi, ok := err.(errors.MultiError); ok {
for _, e := range multi.Errors() {
if found := findAuthError(e); found != nil {
return found
}
}
return nil
}

var httpErr *ErrUnexpectedStatusCode
if errors.As(err, &httpErr) && (httpErr.Code == 401 || httpErr.Code == 403) {
return httpErr
}
return nil
}

// uploadHints builds hint paragraphs for the Sourcegraph access token,
// code host tokens, and a docs link.
func uploadHints(accessToken string, isUnauthorized, isForbidden bool) string {
var causes []string

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

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

needsGitHubToken := strings.HasPrefix(codeintelUploadFlags.repo, "github.com")
needsGitLabToken := strings.HasPrefix(codeintelUploadFlags.repo, "gitlab.com")
var parts []string
parts = append(parts, "Possible causes:\n"+strings.Join(causes, "\n"))
parts = append(parts, "For more details on uploading SCIP indexes, see https://sourcegraph.com/docs/cli/references/code-intel/upload.")

if needsGitHubToken {
if codeintelUploadFlags.gitHubToken != "" {
actionableHints = append(actionableHints,
fmt.Sprintf("The supplied -github-token does not indicate that you have collaborator access to %s.", codeintelUploadFlags.repo),
"Please check the value of the supplied token and its permissions on the code host and try again.",
)
} else {
actionableHints = append(actionableHints,
fmt.Sprintf("Please retry your request with a -github-token=XXX with collaborator access to %s.", codeintelUploadFlags.repo),
"This token will be used to check with the code host that the uploading user has write access to the target repository.",
)
return strings.Join(parts, "\n\n")
}

// sourcegraphAccessTokenHint returns a hint about the Sourcegraph access token
// based on the error type and token state.
func sourcegraphAccessTokenHint(accessToken string, isUnauthorized, isForbidden bool) string {
if isUnauthorized {
if accessToken == "" {
return "No Sourcegraph access token was provided. Set the SRC_ACCESS_TOKEN environment variable to a valid token."
}
} else if needsGitLabToken {
if codeintelUploadFlags.gitLabToken != "" {
actionableHints = append(actionableHints,
fmt.Sprintf("The supplied -gitlab-token does not indicate that you have write access to %s.", codeintelUploadFlags.repo),
"Please check the value of the supplied token and its permissions on the code host and try again.",
)
} else {
actionableHints = append(actionableHints,
fmt.Sprintf("Please retry your request with a -gitlab-token=XXX with write access to %s.", codeintelUploadFlags.repo),
"This token will be used to check with the code host that the uploading user has write access to the target repository.",
)
if _, parseErr := accesstoken.ParsePersonalAccessToken(accessToken); parseErr != nil {
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?"
}
} else {
actionableHints = append(actionableHints,
"Verification is supported for the following code hosts: github.com, gitlab.com.",
"Please request support for additional code host verification at https://github.com/sourcegraph/sourcegraph/issues/4967.",
)
return "The Sourcegraph access token may be invalid, expired, or you may be connecting to the wrong Sourcegraph instance."
}
if isForbidden {
return "You may not have sufficient permissions on this Sourcegraph instance."
}
return ""
}

return errorWithHint{err: originalError, hint: strings.Join(mergeStringSlices(
[]string{"This Sourcegraph instance has enforced auth for SCIP uploads."},
actionableHints,
[]string{"For more details, see https://docs.sourcegraph.com/cli/references/code-intel/upload."},
), "\n")}
// codeHostTokenHints returns hints about GitHub or GitLab tokens.
func codeHostTokenHints(isUnauthorized bool) []string {
if codeintelUploadFlags.gitHubToken != "" || strings.HasPrefix(codeintelUploadFlags.repo, "github.com") {
return []string{gitHubTokenHint(isUnauthorized)}
}
if codeintelUploadFlags.gitLabToken != "" || strings.HasPrefix(codeintelUploadFlags.repo, "gitlab.com") {
return []string{gitLabTokenHint(isUnauthorized)}
}
return []string{"Code host verification is supported for github.com and gitlab.com repositories."}
}

// emergencyOutput creates a default Output object writing to standard out.
func emergencyOutput() *output.Output {
return output.NewOutput(os.Stdout, output.OutputOpts{})
// gitHubTokenHint returns a hint about the GitHub token.
// Only called when gitHubToken is set or repo starts with "github.com".
func gitHubTokenHint(isUnauthorized bool) string {
if codeintelUploadFlags.gitHubToken == "" {
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)
}
if isUnauthorized {
return "The supplied -github-token may be invalid."
}
return "The supplied -github-token may lack the required permissions."
}

func mergeStringSlices(ss ...[]string) []string {
var combined []string
for _, s := range ss {
combined = append(combined, s...)
// gitLabTokenHint returns a hint about the GitLab token.
// Only called when gitLabToken is set or repo starts with "gitlab.com".
func gitLabTokenHint(isUnauthorized bool) string {
if codeintelUploadFlags.gitLabToken == "" {
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)
}
if isUnauthorized {
return "The supplied -gitlab-token may be invalid."
}
return "The supplied -gitlab-token may lack the required permissions."
}

// uploadFailureReason returns the server's response body if available, or a
// generic reason derived from the HTTP status code.
func uploadFailureReason(httpErr *ErrUnexpectedStatusCode) string {
if httpErr.Body != "" {
return httpErr.Body
}
if httpErr.Code == 401 {
return "unauthorized"
}
return "forbidden"
}

return combined
// emergencyOutput creates a default Output object writing to standard out.
func emergencyOutput() *output.Output {
return output.NewOutput(os.Stdout, output.OutputOpts{})
}
Loading
Loading