Skip to content

fix: improve codersdk error messages when not JSON #4495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
chore: tests for codersdk readBodyAsError
  • Loading branch information
deansheather committed Oct 18, 2022
commit e8f2b13cfb7affcfe686faf59b5f9ec0750edba3
6 changes: 4 additions & 2 deletions codersdk/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,12 @@ func readBodyAsError(res *http.Response) error {
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)
Expand Down
200 changes: 200 additions & 0 deletions codersdk/client_internal_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
87 changes: 0 additions & 87 deletions codersdk/client_test.go

This file was deleted.