diff --git a/cli/cliui/provisionerjob.go b/cli/cliui/provisionerjob.go index c781be74f2cc1..1fb7025f2cbb4 100644 --- a/cli/cliui/provisionerjob.go +++ b/cli/cliui/provisionerjob.go @@ -134,7 +134,7 @@ func ProvisionerJob(ctx context.Context, writer io.Writer, opts ProvisionerJobOp logs, closer, err := opts.Logs() if err != nil { - return xerrors.Errorf("logs: %w", err) + return xerrors.Errorf("begin streaming logs: %w", err) } defer closer.Close() diff --git a/coderd/templateversions.go b/coderd/templateversions.go index ccc1cd976111b..e434482ad8b69 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -421,7 +421,7 @@ func (api *API) fetchTemplateVersionDryRunJob(rw http.ResponseWriter, r *http.Re return database.ProvisionerJob{}, false } - // Do a workspace resource check since it's basically a workspace dry-run . + // Do a workspace resource check since it's basically a workspace dry-run. if !api.Authorize(r, rbac.ActionRead, rbac.ResourceWorkspace.InOrg(templateVersion.OrganizationID).WithOwner(job.InitiatorID.String())) { httpapi.Forbidden(rw) diff --git a/codersdk/client.go b/codersdk/client.go index 0c3d89a92843b..a5372dee9e500 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "mime" "net/http" "net/url" "strings" @@ -119,33 +120,54 @@ func readBodyAsError(res *http.Response) error { helper = "Try logging in using 'coder login '." } - if strings.HasPrefix(contentType, "text/plain") { - resp, err := io.ReadAll(res.Body) - if err != nil { - return xerrors.Errorf("read body: %w", err) + resp, err := io.ReadAll(res.Body) + if err != nil { + return xerrors.Errorf("read body: %w", err) + } + + mimeType, _, err := mime.ParseMediaType(contentType) + if err != nil { + mimeType = strings.TrimSpace(strings.Split(contentType, ";")[0]) + } + if mimeType != "application/json" { + if len(resp) > 1024 { + resp = append(resp[:1024], []byte("...")...) + } + if len(resp) == 0 { + resp = []byte("no response body") } return &Error{ statusCode: res.StatusCode, Response: Response{ - Message: string(resp), + Message: "unexpected non-JSON response", + Detail: string(resp), }, Helper: helper, } } - //nolint:varnamelen var m Response - err := json.NewDecoder(res.Body).Decode(&m) + err = json.NewDecoder(bytes.NewBuffer(resp)).Decode(&m) if err != nil { if errors.Is(err, io.EOF) { - // If no body is sent, we'll just provide the status code. return &Error{ statusCode: res.StatusCode, - Helper: helper, + Response: Response{ + Message: "empty response body", + }, + Helper: helper, } } return xerrors.Errorf("decode body: %w", err) } + if m.Message == "" { + if len(resp) > 1024 { + resp = append(resp[:1024], []byte("...")...) + } + m.Message = fmt.Sprintf("unexpected status code %d, response has no message", res.StatusCode) + m.Detail = string(resp) + } + return &Error{ Response: m, statusCode: res.StatusCode, diff --git a/codersdk/client_internal_test.go b/codersdk/client_internal_test.go new file mode 100644 index 0000000000000..f4cb49297c0a0 --- /dev/null +++ b/codersdk/client_internal_test.go @@ -0,0 +1,200 @@ +package codersdk + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" +) + +const ( + jsonCT = "application/json" +) + +func Test_readBodyAsError(t *testing.T) { + t.Parallel() + + exampleURL := "http://example.com" + simpleResponse := Response{ + Message: "test", + Detail: "hi", + } + + longResponse := "" + for i := 0; i < 2000; i++ { + longResponse += "a" + } + + unexpectedJSON := marshalJSON(map[string]any{ + "hello": "world", + "foo": "bar", + }) + + //nolint:bodyclose + tests := []struct { + name string + req *http.Request + res *http.Response + assert func(t *testing.T, err error) + }{ + { + name: "JSONWithRequest", + req: httptest.NewRequest(http.MethodGet, exampleURL, nil), + res: newResponse(http.StatusNotFound, jsonCT, marshalJSON(simpleResponse)), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Equal(t, simpleResponse, sdkErr.Response) + assert.ErrorContains(t, err, sdkErr.Response.Message) + assert.ErrorContains(t, err, sdkErr.Response.Detail) + + assert.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + assert.ErrorContains(t, err, strconv.Itoa(sdkErr.StatusCode())) + + assert.Equal(t, http.MethodGet, sdkErr.method) + assert.ErrorContains(t, err, sdkErr.method) + + assert.Equal(t, exampleURL, sdkErr.url) + assert.ErrorContains(t, err, sdkErr.url) + + assert.Empty(t, sdkErr.Helper) + }, + }, + { + name: "JSONWithoutRequest", + req: nil, + res: newResponse(http.StatusNotFound, jsonCT, marshalJSON(simpleResponse)), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Equal(t, simpleResponse, sdkErr.Response) + assert.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + assert.Empty(t, sdkErr.method) + assert.Empty(t, sdkErr.url) + assert.Empty(t, sdkErr.Helper) + }, + }, + { + name: "UnauthorizedHelper", + req: nil, + res: newResponse(http.StatusUnauthorized, jsonCT, marshalJSON(simpleResponse)), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Contains(t, sdkErr.Helper, "Try logging in") + assert.ErrorContains(t, err, sdkErr.Helper) + }, + }, + { + name: "NonJSON", + req: nil, + res: newResponse(http.StatusNotFound, "text/plain; charset=utf-8", "hello world"), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Contains(t, sdkErr.Response.Message, "unexpected non-JSON response") + assert.Equal(t, "hello world", sdkErr.Response.Detail) + }, + }, + { + name: "NonJSONLong", + req: nil, + res: newResponse(http.StatusNotFound, "text/plain; charset=utf-8", longResponse), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Contains(t, sdkErr.Response.Message, "unexpected non-JSON response") + + expected := longResponse[0:1024] + "..." + assert.Equal(t, expected, sdkErr.Response.Detail) + }, + }, + { + name: "JSONNoBody", + req: nil, + res: newResponse(http.StatusNotFound, jsonCT, ""), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Contains(t, sdkErr.Response.Message, "empty response body") + }, + }, + { + name: "JSONNoMessage", + req: nil, + res: newResponse(http.StatusNotFound, jsonCT, unexpectedJSON), + assert: func(t *testing.T, err error) { + sdkErr := assertSDKError(t, err) + + assert.Contains(t, sdkErr.Response.Message, "unexpected status code") + assert.Contains(t, sdkErr.Response.Message, "has no message") + assert.Equal(t, unexpectedJSON, sdkErr.Response.Detail) + }, + }, + } + + for _, c := range tests { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + c.res.Request = c.req + + err := readBodyAsError(c.res) + c.assert(t, err) + }) + } +} + +func assertSDKError(t *testing.T, err error) *Error { + t.Helper() + + var sdkErr *Error + require.Error(t, err) + require.True(t, xerrors.As(err, &sdkErr)) + + return sdkErr +} + +func newResponse(status int, contentType string, body interface{}) *http.Response { + var r io.ReadCloser + switch v := body.(type) { + case string: + r = io.NopCloser(strings.NewReader(v)) + case []byte: + r = io.NopCloser(bytes.NewReader(v)) + case io.ReadCloser: + r = v + case io.Reader: + r = io.NopCloser(v) + default: + panic(fmt.Sprintf("unknown body type: %T", body)) + } + + return &http.Response{ + Status: http.StatusText(status), + StatusCode: status, + Header: http.Header{ + "Content-Type": []string{contentType}, + }, + Body: r, + } +} + +func marshalJSON(res any) string { + b, err := json.Marshal(res) + if err != nil { + panic(err) + } + + return string(b) +}