diff --git a/cli/errors.go b/cli/errors.go index 12567e0400ac5..ee12ca036af24 100644 --- 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,6 +70,17 @@ func (RootCmd) errorExample() *clibase.Cmd { // A multi-error { Use: "multi-error", + Handler: func(inv *clibase.Invocation) error { + 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. @@ -72,7 +88,10 @@ func (RootCmd) errorExample() *clibase.Cmd { _ = 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 index cef7df19b366d..8b8784735d96d 100644 --- 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 +// 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 "", 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 { // // 2. // -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 index 05ca69fba64f8..d7ca661adff4c 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -323,6 +323,17 @@ 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 { @@ -330,7 +341,6 @@ func ReadBodyAsError(res *http.Response) error { 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 index 29d5a0cd3cf89..0f210bacd2d36 100644 --- 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 index 9ad8f16764c28..438ed664d2bc2 100644 --- 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)