CU-2458: Display server error messages for failed uploads

Author: chrapkowski-sg

cmd/src/code_intel_upload.go

@@ -1,12 +1,14 @@
 package main
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"encoding/json"
 	"flag"
 	"fmt"
 	"io"
+	"net/http"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -15,7 +17,6 @@ import (
 
 	"github.com/pkg/browser"
 
-	"github.com/sourcegraph/sourcegraph/lib/accesstoken"
 	"github.com/sourcegraph/sourcegraph/lib/codeintel/upload"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/sourcegraph/sourcegraph/lib/output"
@@ -88,18 +89,19 @@ func handleCodeIntelUpload(args []string) error {
 		}
 	}
 	if err != nil {
-		return handleUploadError(cfg.AccessToken, err)
+		return handleUploadError(err, "")
 	}
 
 	client := api.NewClient(api.ClientOpts{
 		Out:   io.Discard,
 		Flags: codeintelUploadFlags.apiFlags,
 	})
 
+	wrappedClient := &errorCapturingClient{inner: client}
 	uploadOptions := codeintelUploadOptions(out, isSCIPAvailable)
-	uploadID, err := upload.UploadIndex(ctx, codeintelUploadFlags.file, client, uploadOptions)
+	uploadID, err := upload.UploadIndex(ctx, codeintelUploadFlags.file, wrappedClient, uploadOptions)
 	if err != nil {
-		return handleUploadError(uploadOptions.SourcegraphInstanceOptions.AccessToken, err)
+		return handleUploadError(err, wrappedClient.lastErrorBody)
 	}
 
 	uploadURL, err := makeCodeIntelUploadURL(uploadID)
@@ -232,15 +234,62 @@ func (e errorWithHint) Error() string {
 	return fmt.Sprintf("%s\n\n%s\n", e.err, e.hint)
 }
 
+// errorCapturingClient wraps an upload.Client to capture the response body
+// from error responses (401, 403, etc.). The pinned lib version discards the
+// body for 401 responses, so we capture it here before the lib sees it.
+type errorCapturingClient struct {
+	inner         upload.Client
+	lastErrorBody string
+}
+
+func (c *errorCapturingClient) Do(req *http.Request) (*http.Response, error) {
+	resp, err := c.inner.Do(req)
+	if err != nil {
+		return resp, err
+	}
+	if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
+		body, readErr := io.ReadAll(resp.Body)
+		resp.Body.Close()
+		if readErr == nil && len(bytes.TrimSpace(body)) > 0 && !bytes.HasPrefix(bytes.TrimSpace(body), []byte{'<'}) {
+			c.lastErrorBody = string(bytes.TrimSpace(body))
+		}
+		// Replace body so the lib can still read it
+		resp.Body = io.NopCloser(bytes.NewReader(body))
+	}
+	return resp, nil
+}
+
 // 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.
-func handleUploadError(accessToken string, err error) error {
+func handleUploadError(err error, serverMessage string) error {
 	if errors.Is(err, upload.ErrUnauthorized) {
-		err = attachHintsForAuthorizationError(accessToken, err)
+		if serverMessage != "" {
+			// The server provided a specific error message (e.g. "must provide gitlab_token").
+			// Show it directly so the user knows what went wrong.
+			err = errorWithHint{
+				err:  errors.Newf("upload rejected by server: %s", serverMessage),
+				hint: codeHostTokenHint(),
+			}
+		} else {
+			// Generic 401 with no server message — likely a missing or invalid Sourcegraph access token.
+			err = errorWithHint{
+				err:  errors.New("upload failed: unauthorized (check your Sourcegraph access token)"),
+				hint: "To create a Sourcegraph access token, see https://sourcegraph.com/docs/cli/how-tos/creating_an_access_token.",
+			}
+		}
+	} else if strings.Contains(err.Error(), "unexpected status code: 403") {
+		// TODO: replace with errors.Is(err, upload.ErrForbidden) once lib snapshot is updated.
+		displayMsg := "upload failed: forbidden"
+		if serverMessage != "" {
+			displayMsg = fmt.Sprintf("upload rejected by server: %s", serverMessage)
+		}
+		err = errorWithHint{
+			err:  errors.New(displayMsg),
+			hint: codeHostTokenHint(),
+		}
 	}
-
 	if codeintelUploadFlags.ignoreUploadFailures {
 		// Report but don't return the error
 		fmt.Println(err.Error())
@@ -250,76 +299,23 @@ func handleUploadError(accessToken string, err error) error {
 	return err
 }
 
-func attachHintsForAuthorizationError(accessToken string, originalError error) error {
-	var actionableHints []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>.")
+// codeHostTokenHint returns documentation links relevant to the upload failure.
+// Always includes the general upload docs, and adds code-host-specific token
+// documentation if the user supplied a code host token flag.
+func codeHostTokenHint() string {
+	hint := "For more details on uploading SCIP indexes, see https://sourcegraph.com/docs/cli/references/code-intel/upload."
+	if codeintelUploadFlags.gitHubToken != "" {
+		hint += "\nIf the issue is related to your GitHub token, see https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens."
 	}
-
-	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/LSIF data."},
-			actionableHints,
-			[]string{"For more details, see https://sourcegraph.com/docs/cli/how-tos/creating_an_access_token."},
-		), "\n")}
+	if codeintelUploadFlags.gitLabToken != "" {
+		hint += "\nIf the issue is related to your GitLab token, see https://docs.gitlab.com/user/profile/personal_access_tokens/."
 	}
-
-	needsGitHubToken := strings.HasPrefix(codeintelUploadFlags.repo, "github.com")
-	needsGitLabToken := strings.HasPrefix(codeintelUploadFlags.repo, "gitlab.com")
-
-	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.",
-			)
-		}
-	} 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.",
-			)
-		}
-	} 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 errorWithHint{err: originalError, hint: strings.Join(mergeStringSlices(
-		[]string{"This Sourcegraph instance has enforced auth for SCIP/LSIF uploads."},
-		actionableHints,
-		[]string{"For more details, see https://docs.sourcegraph.com/cli/references/code-intel/upload."},
-	), "\n")}
+	return hint
 }
 
 // emergencyOutput creates a default Output object writing to standard out.
 func emergencyOutput() *output.Output {
 	return output.NewOutput(os.Stdout, output.OutputOpts{})
 }
 
-func mergeStringSlices(ss ...[]string) []string {
-	var combined []string
-	for _, s := range ss {
-		combined = append(combined, s...)
-	}
 
-	return combined
-}