Skip to content

Commit

Permalink
updating errors coverage to 100% (#400)
Browse files Browse the repository at this point in the history
  • Loading branch information
@luthermonson
luthermonson committed Oct 21, 2023
1 parent f36e2e3 commit d209855
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 86 deletions.
54 changes: 29 additions & 25 deletions errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,41 +47,45 @@ type APIError struct {

func coupleAPIErrors(r *resty.Response, err error) (*resty.Response, error) {
if err != nil {
// an error was raised in go code, no need to check the resty Response
return nil, NewError(err)
}

if r.Error() != nil {
// Check that response is of the correct content-type before unmarshalling
expectedContentType := r.Request.Header.Get("Accept")
responseContentType := r.Header().Get("Content-Type")
if r.Error() == nil {
// no error in the resty Response
return r, nil
}

// If the upstream Linode API server being fronted fails to respond to the request,
// the http server will respond with a default "Bad Gateway" page with Content-Type
// "text/html".
if r.StatusCode() == http.StatusBadGateway && responseContentType == "text/html" {
return nil, Error{Code: http.StatusBadGateway, Message: http.StatusText(http.StatusBadGateway)}
}
// handle the resty Response errors

if responseContentType != expectedContentType {
msg := fmt.Sprintf(
"Unexpected Content-Type: Expected: %v, Received: %v\nResponse body: %s",
expectedContentType,
responseContentType,
string(r.Body()),
)
// Check that response is of the correct content-type before unmarshalling
expectedContentType := r.Request.Header.Get("Accept")
responseContentType := r.Header().Get("Content-Type")

return nil, Error{Code: r.StatusCode(), Message: msg}
}
// If the upstream Linode API server being fronted fails to respond to the request,
// the http server will respond with a default "Bad Gateway" page with Content-Type
// "text/html".
if r.StatusCode() == http.StatusBadGateway && responseContentType == "text/html" {
return nil, Error{Code: http.StatusBadGateway, Message: http.StatusText(http.StatusBadGateway)}
}

apiError, ok := r.Error().(*APIError)
if !ok || (ok && len(apiError.Errors) == 0) {
return r, nil
}
if responseContentType != expectedContentType {
msg := fmt.Sprintf(
"Unexpected Content-Type: Expected: %v, Received: %v\nResponse body: %s",
expectedContentType,
responseContentType,
string(r.Body()),
)

return nil, Error{Code: r.StatusCode(), Message: msg}
}

return nil, NewError(r)
apiError, ok := r.Error().(*APIError)
if !ok || (ok && len(apiError.Errors) == 0) {
return r, nil
}

return r, nil
return nil, NewError(r)
}

func (e APIError) Error() string {
Expand Down
184 changes: 123 additions & 61 deletions errors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,82 @@ import (
"github.com/google/go-cmp/cmp"
)

type tstringer string
type testStringer string

func (t tstringer) String() string {
func (t testStringer) String() string {
return string(t)
}

type testError string

func (e testError) Error() string {
return string(e)
}

func restyError(reason, field string) *resty.Response {
var reasons []APIErrorReason

// allow for an empty reasons
if reason != "" && field != "" {
reasons = append(reasons, APIErrorReason{
Reason: reason,
Field: field,
})
}

return &resty.Response{
RawResponse: &http.Response{
StatusCode: 500,
},
Request: &resty.Request{
Error: &APIError{
Errors: reasons,
},
},
}
}

func TestNewError(t *testing.T) {
if NewError(nil) != nil {
t.Errorf("nil error should return nil")
}
if NewError(struct{}{}).Code != ErrorUnsupported {
t.Error("empty struct should return unsupported error type")
}

err := errors.New("test")
newErr := NewError(&err)
if newErr.Message == err.Error() && newErr.Code == ErrorFromError {
t.Error("nil error should return nil")
newErr := NewError(err)

if newErr.Message != err.Error() && newErr.Code != ErrorFromError {
t.Error("error should return ErrorFromError")
}

if err := NewError(&resty.Response{Request: &resty.Request{}}); err.Message != "Unexpected Resty Error Response, no error" {
t.Error("Unexpected Resty Error Response, no error")
if newErr.Error() != "[002] test" {
t.Error("Error should support Error() formatter with code")
}

rerr := &resty.Response{
RawResponse: &http.Response{
StatusCode: 500,
},
Request: &resty.Request{
Error: &APIError{
[]APIErrorReason{
{
Reason: "testreason",
Field: "testfield",
},
},
},
},
if NewError(newErr) != newErr {
t.Error("Error should be itself")
}

if err := NewError(&resty.Response{Request: &resty.Request{}}); err.Message != "Unexpected Resty Error Response, no error" {
t.Error("Unexpected Resty Error Response, no error")
}

if err := NewError(rerr); err.Message != "[testfield] testreason" {
if err := NewError(restyError("testreason", "testfield")); err.Message != "[testfield] testreason" {
t.Error("rest response error should should be set")
}

if err := NewError("stringerror"); err.Message != "stringerror" || err.Code != ErrorFromString {
t.Errorf("string error should be set")
}

if err := NewError(tstringer("teststringer")); err.Message != "teststringer" || err.Code != ErrorFromStringer {
t.Errorf("stringer error should be set")
if err := NewError(testStringer("teststringer")); err.Message != "teststringer" || err.Code != ErrorFromStringer {
t.Errorf("error should be set for a stringer interface")
}

if err := NewError(testError("testerror")); err.Message != "testerror" || err.Code != ErrorFromError {
t.Errorf("error should be set for an error interface")
}
}

Expand All @@ -82,59 +109,94 @@ func createTestServer(method, route, contentType, body string, statusCode int) (
return ts, &client
}

func TestCoupleAPIErrors_genericHtmlError(t *testing.T) {
rawResponse := `<html>
func TestCoupleAPIErrors(t *testing.T) {
t.Run("not nil error generates error", func(t *testing.T) {
err := errors.New("test")
if _, err := coupleAPIErrors(nil, err); !cmp.Equal(err, NewError(err)) {
t.Errorf("expect a not nil error to be returned as an Error")
}
})

t.Run("resty 500 response error with reasons", func(t *testing.T) {
if _, err := coupleAPIErrors(restyError("testreason", "testfield"), nil); err.Error() != "[500] [testfield] testreason" {
t.Error("resty error should return with proper format [code] [field] reason")
}
})

t.Run("resty 500 response error without reasons", func(t *testing.T) {
if _, err := coupleAPIErrors(restyError("", ""), nil); err != nil {
t.Error("resty error with no reasons should return no error")
}
})

t.Run("resty response with nil error", func(t *testing.T) {
emptyErr := &resty.Response{
RawResponse: &http.Response{
StatusCode: 500,
},
Request: &resty.Request{
Error: nil,
},
}
if _, err := coupleAPIErrors(emptyErr, nil); err != nil {
t.Error("resty error with no reasons should return no error")
}
})

t.Run("generic html error", func(t *testing.T) {
rawResponse := `<html>
<head><title>500 Internal Server Error</title></head>
<body bgcolor="white">
<center><h1>500 Internal Server Error</h1></center>
<hr><center>nginx</center>
</body>
</html>`
route := "/v4/linode/instances/123"
ts, client := createTestServer(http.MethodGet, route, "text/html", rawResponse, http.StatusInternalServerError)
client.SetDebug(true)
defer ts.Close()

expectedError := Error{
Code: http.StatusInternalServerError,
Message: "Unexpected Content-Type: Expected: application/json, Received: text/html\nResponse body: " + rawResponse,
}
route := "/v4/linode/instances/123"
ts, client := createTestServer(http.MethodGet, route, "text/html", rawResponse, http.StatusInternalServerError)
// client.SetDebug(true)
defer ts.Close()

expectedError := Error{
Code: http.StatusInternalServerError,
Message: "Unexpected Content-Type: Expected: application/json, Received: text/html\nResponse body: " + rawResponse,
}

_, err := coupleAPIErrors(client.R(context.Background()).SetResult(&Instance{}).Get(ts.URL + route))
if diff := cmp.Diff(expectedError, err); diff != "" {
t.Errorf("expected error to match but got diff:\n%s", diff)
}
}
_, err := coupleAPIErrors(client.R(context.Background()).SetResult(&Instance{}).Get(ts.URL + route))
if diff := cmp.Diff(expectedError, err); diff != "" {
t.Errorf("expected error to match but got diff:\n%s", diff)
}
})

func TestCoupleAPIErrors_badGatewayError(t *testing.T) {
rawResponse := []byte(`<html>
t.Run("bad gateway error", func(t *testing.T) {
rawResponse := []byte(`<html>
<head><title>502 Bad Gateway</title></head>
<body bgcolor="white">
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx</center>
</body>
</html>`)
buf := io.NopCloser(bytes.NewBuffer(rawResponse))
buf := io.NopCloser(bytes.NewBuffer(rawResponse))

resp := &resty.Response{
Request: &resty.Request{
Error: errors.New("Bad Gateway"),
},
RawResponse: &http.Response{
Header: http.Header{
"Content-Type": []string{"text/html"},
resp := &resty.Response{
Request: &resty.Request{
Error: errors.New("Bad Gateway"),
},
StatusCode: http.StatusBadGateway,
Body: buf,
},
}
RawResponse: &http.Response{
Header: http.Header{
"Content-Type": []string{"text/html"},
},
StatusCode: http.StatusBadGateway,
Body: buf,
},
}

expectedError := Error{
Code: http.StatusBadGateway,
Message: http.StatusText(http.StatusBadGateway),
}
expectedError := Error{
Code: http.StatusBadGateway,
Message: http.StatusText(http.StatusBadGateway),
}

if _, err := coupleAPIErrors(resp, nil); !cmp.Equal(err, expectedError) {
t.Errorf("expected error %#v to match error %#v", err, expectedError)
}
if _, err := coupleAPIErrors(resp, nil); !cmp.Equal(err, expectedError) {
t.Errorf("expected error %#v to match error %#v", err, expectedError)
}
})
}

0 comments on commit d209855

Please sign in to comment.