coder · Emyrk · Jan 11, 2024 · Jan 11, 2024 · Jan 11, 2024 · Jan 11, 2024
diff --git a/cli/errors.go b/cli/errors.go
@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -43,6 +44,10 @@ func (RootCmd) errorExample() *clibase.Cmd {
 	//nolint:errorlint,forcetypeassert
 	apiError.(*codersdk.Error).Helper = "Have you tried turning it off and on again?"
 
+	//nolint:errorlint,forcetypeassert
+	apiErrorNoHelper := apiError.(*codersdk.Error)
+	apiErrorNoHelper.Helper = ""
+
 	// Some flags
 	var magicWord clibase.String
 
@@ -65,14 +70,28 @@ func (RootCmd) errorExample() *clibase.Cmd {
 			// A multi-error
 			{
 				Use: "multi-error",
+				Handler: func(inv *clibase.Invocation) error {
 Hidden: true, 
 Hidden: true, 
+					return xerrors.Errorf("wrapped: %w", errors.Join(
+						xerrors.Errorf("first error: %w", errorWithStackTrace()),
+						xerrors.Errorf("second error: %w", errorWithStackTrace()),
+						xerrors.Errorf("wrapped api error: %w", apiErrorNoHelper),
+					))
+				},
+			},
+			{
+				Use:   "multi-multi-error",
+				Short: "This is a multi error inside a multi error",
 				Handler: func(inv *clibase.Invocation) error {
 					// Closing the stdin file descriptor will cause the next close
 					// to fail. This is joined to the returned Command error.
 					if f, ok := inv.Stdin.(*os.File); ok {
 						_ = f.Close()
 					}
 
-					return xerrors.Errorf("some error: %w", errorWithStackTrace())
+					return errors.Join(
+						xerrors.Errorf("first error: %w", errorWithStackTrace()),
+						xerrors.Errorf("second error: %w", errorWithStackTrace()),
+					)
 				},
 			},
 

diff --git a/cli/root.go b/cli/root.go
@@ -1016,7 +1016,7 @@ type prettyErrorFormatter struct {
 // format formats the error to the console. This error should be human
 // readable.
 func (p *prettyErrorFormatter) format(err error) {
-	output := cliHumanFormatError(err, &formatOpts{
+	output, _ := cliHumanFormatError("", err, &formatOpts{
 		Verbose: p.verbose,
 	})
 	// always trail with a newline
@@ -1030,41 +1030,66 @@ type formatOpts struct {
 const indent = "    "
 
 // cliHumanFormatError formats an error for the CLI. Newlines and styling are
-// included.
-func cliHumanFormatError(err error, opts *formatOpts) string {
+// included. The second return value is true if the error is special and the error
+// chain has custom formatting applied.
+//
+// If you change this code, you can use the cli "example-errors" tool to
+// verify all errors still look ok.
+//
+//	go run main.go exp example-error <type>
+//	       go run main.go exp example-error api
+//	       go run main.go exp example-error cmd
+//	       go run main.go exp example-error multi-error
+//	       go run main.go exp example-error validation
+//
+//nolint:errorlint
+func cliHumanFormatError(from string, err error, opts *formatOpts) (string, bool) {
 	if opts == nil {
 		opts = &formatOpts{}
 	}
+	if err == nil {
+		return "<nil>", true
+	}
 
-	//nolint:errorlint
 	if multi, ok := err.(interface{ Unwrap() []error }); ok {
 		multiErrors := multi.Unwrap()
 		if len(multiErrors) == 1 {
 			// Format as a single error
-			return cliHumanFormatError(multiErrors[0], opts)
+			return cliHumanFormatError(from, multiErrors[0], opts)
 		}
-		return formatMultiError(multiErrors, opts)
+		return formatMultiError(from, multiErrors, opts), true
 	}
 
 	// First check for sentinel errors that we want to handle specially.
 	// Order does matter! We want to check for the most specific errors first.
-	var sdkError *codersdk.Error
-	if errors.As(err, &sdkError) {
-		return formatCoderSDKError(sdkError, opts)
+	if sdkError, ok := err.(*codersdk.Error); ok {
+		return formatCoderSDKError(from, sdkError, opts), true
 	}
 
-	var cmdErr *clibase.RunCommandError
-	if errors.As(err, &cmdErr) {
-		return formatRunCommandError(cmdErr, opts)
+	if cmdErr, ok := err.(*clibase.RunCommandError); ok {
+		// no need to pass the "from" context to this since it is always
+		// top level. We care about what is below this.
+		return formatRunCommandError(cmdErr, opts), true
 	}
 
+	uw, ok := err.(interface{ Unwrap() error })
+	if ok {
+		msg, special := cliHumanFormatError(from+traceError(err), uw.Unwrap(), opts)
+		if special {
+			return msg, special
+		}
+	}
+	// If we got here, that means that the wrapped error chain does not have
+	// any special formatting below it. So we want to return the topmost non-special
+	// error (which is 'err')
+
 	// Default just printing the error. Use +v for verbose to handle stack
 	// traces of xerrors.
 	if opts.Verbose {
-		return pretty.Sprint(headLineStyle(), fmt.Sprintf("%+v", err))
+		return pretty.Sprint(headLineStyle(), fmt.Sprintf("%+v", err)), false
 	}
 
-	return pretty.Sprint(headLineStyle(), fmt.Sprintf("%v", err))
+	return pretty.Sprint(headLineStyle(), fmt.Sprintf("%v", err)), false
 }
 
 // formatMultiError formats a multi-error. It formats it as a list of errors.
@@ -1075,15 +1100,20 @@ func cliHumanFormatError(err error, opts *formatOpts) string {
 //		   <verbose error message>
 //		2. <heading error message>
 //		   <verbose error message>
-func formatMultiError(multi []error, opts *formatOpts) string {
+func formatMultiError(from string, multi []error, opts *formatOpts) string {
 	var errorStrings []string
 	for _, err := range multi {
-		errorStrings = append(errorStrings, cliHumanFormatError(err, opts))
+		msg, _ := cliHumanFormatError("", err, opts)
+		errorStrings = append(errorStrings, msg)
 	}
 
 	// Write errors out
 	var str strings.Builder
-	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("%d errors encountered:", len(multi))))
+	var traceMsg string
+	if from != "" {
+		traceMsg = fmt.Sprintf("Trace=[%s])", from)
+	}
+	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("%d errors encountered: %s", len(multi), traceMsg)))
 	for i, errStr := range errorStrings {
 		// Indent each error
 		errStr = strings.ReplaceAll(errStr, "\n", "\n"+indent)
@@ -1112,24 +1142,30 @@ func formatRunCommandError(err *clibase.RunCommandError, opts *formatOpts) strin
 	var str strings.Builder
 	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Encountered an error running %q", err.Cmd.FullName())))
 
-	msgString := fmt.Sprintf("%v", err.Err)
-	if opts.Verbose {
-		// '%+v' includes stack traces
-		msgString = fmt.Sprintf("%+v", err.Err)
-	}
+	msgString, special := cliHumanFormatError("", err.Err, opts)
 	_, _ = str.WriteString("\n")
-	_, _ = str.WriteString(pretty.Sprint(tailLineStyle(), msgString))
+	if special {
+		_, _ = str.WriteString(msgString)
+	} else {
+		_, _ = str.WriteString(pretty.Sprint(tailLineStyle(), msgString))
+	}
+
 	return str.String()
 }
 
 // formatCoderSDKError come from API requests. In verbose mode, add the
 // request debug information.
-func formatCoderSDKError(err *codersdk.Error, opts *formatOpts) string {
+func formatCoderSDKError(from string, err *codersdk.Error, opts *formatOpts) string {
 	var str strings.Builder
 	if opts.Verbose {
 		_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("API request error to \"%s:%s\". Status code %d", err.Method(), err.URL(), err.StatusCode())))
 		_, _ = str.WriteString("\n")
 	}
+	// Always include this trace. Users can ignore this.
+	if from != "" {
+		_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Trace=[%s]", from)))
+		_, _ = str.WriteString("\n")
+	}
 
 	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), err.Message))
 	if err.Helper != "" {
@@ -1144,6 +1180,21 @@ func formatCoderSDKError(err *codersdk.Error, opts *formatOpts) string {
 	return str.String()
 }
 
+// traceError is a helper function that aides developers debugging failed cli
+// commands. When we pretty print errors, we lose the context in which they came.
+// This function adds the context back. Unfortunately there is no easy way to get
+// the prefix to: "error string: %w", so we do a bit of string manipulation.
+//
+//nolint:errorlint
+func traceError(err error) string {
+	if uw, ok := err.(interface{ Unwrap() error }); ok {
+		a, b := err.Error(), uw.Unwrap().Error()
+		c := strings.TrimSuffix(a, b)
+		return c
+	}
+	return err.Error()
+}
+
 // These styles are arbitrary.
 func headLineStyle() pretty.Style {
 	return cliui.DefaultStyles.Error

diff --git a/codersdk/client.go b/codersdk/client.go
@@ -323,14 +323,24 @@ func (c *Client) Request(ctx context.Context, method, path string, body interfac
 	return resp, err
 }
 
+// ExpectJSONMime is a helper function that will assert the content type
+// of the response is application/json.
+func ExpectJSONMime(res *http.Response) error {
+	contentType := res.Header.Get("Content-Type")
+	mimeType := parseMimeType(contentType)
+	if mimeType != "application/json" {
+		return xerrors.Errorf("unexpected non-JSON response %q", contentType)
+	}
+	return nil
+}
+
 // ReadBodyAsError reads the response as a codersdk.Response, and
 // wraps it in a codersdk.Error type for easy marshaling.
 func ReadBodyAsError(res *http.Response) error {
 	if res == nil {
 		return xerrors.Errorf("no body returned")
 	}
 	defer res.Body.Close()
-	contentType := res.Header.Get("Content-Type")
 
 	var requestMethod, requestURL string
 	if res.Request != nil {
@@ -352,8 +362,7 @@ func ReadBodyAsError(res *http.Response) error {
 		return xerrors.Errorf("read body: %w", err)
 	}
 
-	mimeType := parseMimeType(contentType)
-	if mimeType != "application/json" {
+	if mimeErr := ExpectJSONMime(res); mimeErr != nil {
 		if len(resp) > 2048 {
 			resp = append(resp[:2048], []byte("...")...)
 		}
@@ -365,7 +374,7 @@ func ReadBodyAsError(res *http.Response) error {
 			method:     requestMethod,
 			url:        requestURL,
 			Response: Response{
-				Message: fmt.Sprintf("unexpected non-JSON response %q", contentType),
+				Message: mimeErr.Error(),
 				Detail:  string(resp),
 			},
 			Helper: helpMessage,

diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -2065,7 +2065,7 @@ func (c *Client) BuildInfo(ctx context.Context) (BuildInfoResponse, error) {
 	}
 	defer res.Body.Close()
 
-	if res.StatusCode != http.StatusOK {
+	if res.StatusCode != http.StatusOK || ExpectJSONMime(res) != nil {
 		return BuildInfoResponse{}, ReadBodyAsError(res)
 	}
 

diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -157,7 +158,10 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
 	// TODO: Probably do some version checking here
 	info, err := client.SDKClient.BuildInfo(ctx)
 	if err != nil {
-		return nil, xerrors.Errorf("failed to fetch build info from %q: %w", opts.DashboardURL, err)
+		return nil, fmt.Errorf("buildinfo: %w", errors.Join(
+			fmt.Errorf("unable to fetch build info from primary coderd. Are you sure %q is a coderd instance?", opts.DashboardURL),
+			err,
+		))
 	}
 	if info.WorkspaceProxy {
 		return nil, xerrors.Errorf("%q is a workspace proxy, not a primary coderd instance", opts.DashboardURL)