Add a way to return the body of a 5xx response. (#484)

expose body when handling HTTP errors

Signed-off-by: Marcin Owsiany <marcin@owsiany.pl>
This commit is contained in:
Marcin Owsiany 2018-10-25 19:44:21 +02:00 committed by Krasi Georgiev
parent 16f375c74d
commit f30f428035
2 changed files with 147 additions and 70 deletions

View File

@ -60,6 +60,8 @@ const (
ErrCanceled = "canceled" ErrCanceled = "canceled"
ErrExec = "execution" ErrExec = "execution"
ErrBadResponse = "bad_response" ErrBadResponse = "bad_response"
ErrServer = "server_error"
ErrClient = "client_error"
// Possible values for HealthStatus. // Possible values for HealthStatus.
HealthGood HealthStatus = "up" HealthGood HealthStatus = "up"
@ -71,6 +73,7 @@ const (
type Error struct { type Error struct {
Type ErrorType Type ErrorType
Msg string Msg string
Detail string
} }
func (e *Error) Error() string { func (e *Error) Error() string {
@ -460,6 +463,16 @@ func apiError(code int) bool {
return code == statusAPIError || code == http.StatusBadRequest return code == statusAPIError || code == http.StatusBadRequest
} }
func errorTypeAndMsgFor(resp *http.Response) (ErrorType, string) {
switch resp.StatusCode / 100 {
case 4:
return ErrClient, fmt.Sprintf("client error: %d", resp.StatusCode)
case 5:
return ErrServer, fmt.Sprintf("server error: %d", resp.StatusCode)
}
return ErrBadResponse, fmt.Sprintf("bad response code %d", resp.StatusCode)
}
func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) { func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) {
resp, body, err := c.Client.Do(ctx, req) resp, body, err := c.Client.Do(ctx, req)
if err != nil { if err != nil {
@ -469,9 +482,11 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [
code := resp.StatusCode code := resp.StatusCode
if code/100 != 2 && !apiError(code) { if code/100 != 2 && !apiError(code) {
errorType, errorMsg := errorTypeAndMsgFor(resp)
return resp, body, &Error{ return resp, body, &Error{
Type: ErrBadResponse, Type: errorType,
Msg: fmt.Sprintf("bad response code %d", resp.StatusCode), Msg: errorMsg,
Detail: string(body),
} }
} }

View File

@ -18,6 +18,7 @@ package v1
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"net/http" "net/http"
"net/url" "net/url"
@ -32,6 +33,7 @@ import (
type apiTest struct { type apiTest struct {
do func() (interface{}, error) do func() (interface{}, error)
inErr error inErr error
inStatusCode int
inRes interface{} inRes interface{}
reqPath string reqPath string
@ -75,7 +77,9 @@ func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Respon
} }
resp := &http.Response{} resp := &http.Response{}
if test.inErr != nil { if test.inStatusCode != 0 {
resp.StatusCode = test.inStatusCode
} else if test.inErr != nil {
resp.StatusCode = statusAPIError resp.StatusCode = statusAPIError
} else { } else {
resp.StatusCode = http.StatusOK resp.StatusCode = http.StatusOK
@ -194,6 +198,42 @@ func TestAPIs(t *testing.T) {
}, },
err: fmt.Errorf("some error"), err: fmt.Errorf("some error"),
}, },
{
do: doQuery("2", testTime),
inRes: "some body",
inStatusCode: 500,
inErr: &Error{
Type: ErrServer,
Msg: "server error: 500",
Detail: "some body",
},
reqMethod: "GET",
reqPath: "/api/v1/query",
reqParam: url.Values{
"query": []string{"2"},
"time": []string{testTime.Format(time.RFC3339Nano)},
},
err: errors.New("server_error: server error: 500"),
},
{
do: doQuery("2", testTime),
inRes: "some body",
inStatusCode: 404,
inErr: &Error{
Type: ErrClient,
Msg: "client error: 404",
Detail: "some body",
},
reqMethod: "GET",
reqPath: "/api/v1/query",
reqParam: url.Values{
"query": []string{"2"},
"time": []string{testTime.Format(time.RFC3339Nano)},
},
err: errors.New("client_error: client error: 404"),
},
{ {
do: doQueryRange("2", Range{ do: doQueryRange("2", Range{
@ -498,29 +538,34 @@ func TestAPIs(t *testing.T) {
var tests []apiTest var tests []apiTest
tests = append(tests, queryTests...) tests = append(tests, queryTests...)
for _, test := range tests { for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
client.curTest = test client.curTest = test
res, err := test.do() res, err := test.do()
if test.err != nil { if test.err != nil {
if err == nil { if err == nil {
t.Errorf("expected error %q but got none", test.err) t.Fatalf("expected error %q but got none", test.err)
continue
} }
if err.Error() != test.err.Error() { if err.Error() != test.err.Error() {
t.Errorf("unexpected error: want %s, got %s", test.err, err) t.Errorf("unexpected error: want %s, got %s", test.err, err)
} }
continue if apiErr, ok := err.(*Error); ok {
if apiErr.Detail != test.inRes {
t.Errorf("%q should be %q", apiErr.Detail, test.inRes)
}
}
return
} }
if err != nil { if err != nil {
t.Errorf("unexpected error: %s", err) t.Fatalf("unexpected error: %s", err)
continue
} }
if !reflect.DeepEqual(res, test.res) { if !reflect.DeepEqual(res, test.res) {
t.Errorf("unexpected result: want %v, got %v", test.res, res) t.Errorf("unexpected result: want %v, got %v", test.res, res)
} }
})
} }
} }
@ -534,8 +579,8 @@ type testClient struct {
type apiClientTest struct { type apiClientTest struct {
code int code int
response interface{} response interface{}
expected string expectedBody string
err *Error expectedErr *Error
} }
func (c *testClient) URL(ep string, args map[string]string) *url.URL { func (c *testClient) URL(ep string, args map[string]string) *url.URL {
@ -575,98 +620,108 @@ func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response,
func TestAPIClientDo(t *testing.T) { func TestAPIClientDo(t *testing.T) {
tests := []apiClientTest{ tests := []apiClientTest{
{ {
code: statusAPIError,
response: &apiResponse{ response: &apiResponse{
Status: "error", Status: "error",
Data: json.RawMessage(`null`), Data: json.RawMessage(`null`),
ErrorType: ErrBadData, ErrorType: ErrBadData,
Error: "failed", Error: "failed",
}, },
err: &Error{ expectedErr: &Error{
Type: ErrBadData, Type: ErrBadData,
Msg: "failed", Msg: "failed",
}, },
code: statusAPIError, expectedBody: `null`,
expected: `null`,
}, },
{ {
code: statusAPIError,
response: &apiResponse{ response: &apiResponse{
Status: "error", Status: "error",
Data: json.RawMessage(`"test"`), Data: json.RawMessage(`"test"`),
ErrorType: ErrTimeout, ErrorType: ErrTimeout,
Error: "timed out", Error: "timed out",
}, },
err: &Error{ expectedErr: &Error{
Type: ErrTimeout, Type: ErrTimeout,
Msg: "timed out", Msg: "timed out",
}, },
code: statusAPIError, expectedBody: `test`,
expected: `test`,
}, },
{ {
response: "bad json",
err: &Error{
Type: ErrBadResponse,
Msg: "bad response code 500",
},
code: http.StatusInternalServerError, code: http.StatusInternalServerError,
response: "500 error details",
expectedErr: &Error{
Type: ErrServer,
Msg: "server error: 500",
Detail: "500 error details",
},
}, },
{ {
code: http.StatusNotFound,
response: "404 error details",
expectedErr: &Error{
Type: ErrClient,
Msg: "client error: 404",
Detail: "404 error details",
},
},
{
code: http.StatusBadRequest,
response: &apiResponse{ response: &apiResponse{
Status: "error", Status: "error",
Data: json.RawMessage(`null`), Data: json.RawMessage(`null`),
ErrorType: ErrBadData, ErrorType: ErrBadData,
Error: "end timestamp must not be before start time", Error: "end timestamp must not be before start time",
}, },
err: &Error{ expectedErr: &Error{
Type: ErrBadData, Type: ErrBadData,
Msg: "end timestamp must not be before start time", Msg: "end timestamp must not be before start time",
}, },
code: http.StatusBadRequest,
}, },
{ {
code: statusAPIError,
response: "bad json", response: "bad json",
err: &Error{ expectedErr: &Error{
Type: ErrBadResponse, Type: ErrBadResponse,
Msg: "invalid character 'b' looking for beginning of value", Msg: "invalid character 'b' looking for beginning of value",
}, },
code: statusAPIError,
}, },
{ {
code: statusAPIError,
response: &apiResponse{ response: &apiResponse{
Status: "success", Status: "success",
Data: json.RawMessage(`"test"`), Data: json.RawMessage(`"test"`),
}, },
err: &Error{ expectedErr: &Error{
Type: ErrBadResponse, Type: ErrBadResponse,
Msg: "inconsistent body for response code", Msg: "inconsistent body for response code",
}, },
code: statusAPIError,
}, },
{ {
code: statusAPIError,
response: &apiResponse{ response: &apiResponse{
Status: "success", Status: "success",
Data: json.RawMessage(`"test"`), Data: json.RawMessage(`"test"`),
ErrorType: ErrTimeout, ErrorType: ErrTimeout,
Error: "timed out", Error: "timed out",
}, },
err: &Error{ expectedErr: &Error{
Type: ErrBadResponse, Type: ErrBadResponse,
Msg: "inconsistent body for response code", Msg: "inconsistent body for response code",
}, },
code: statusAPIError,
}, },
{ {
code: http.StatusOK,
response: &apiResponse{ response: &apiResponse{
Status: "error", Status: "error",
Data: json.RawMessage(`"test"`), Data: json.RawMessage(`"test"`),
ErrorType: ErrTimeout, ErrorType: ErrTimeout,
Error: "timed out", Error: "timed out",
}, },
err: &Error{ expectedErr: &Error{
Type: ErrBadResponse, Type: ErrBadResponse,
Msg: "inconsistent body for response code", Msg: "inconsistent body for response code",
}, },
code: http.StatusOK,
}, },
} }
@ -677,30 +732,37 @@ func TestAPIClientDo(t *testing.T) {
} }
client := &apiClient{tc} client := &apiClient{tc}
for _, test := range tests { for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
tc.ch <- test tc.ch <- test
_, body, err := client.Do(context.Background(), tc.req) _, body, err := client.Do(context.Background(), tc.req)
if test.err != nil { if test.expectedErr != nil {
if err == nil { if err == nil {
t.Errorf("expected error %q but got none", test.err) t.Fatalf("expected error %q but got none", test.expectedErr)
continue
} }
if test.err.Error() != err.Error() { if test.expectedErr.Error() != err.Error() {
t.Errorf("unexpected error: want %q, got %q", test.err, err) t.Errorf("unexpected error: want %q, got %q", test.expectedErr, err)
} }
continue if test.expectedErr.Detail != "" {
apiErr := err.(*Error)
if apiErr.Detail != test.expectedErr.Detail {
t.Errorf("unexpected error details: want %q, got %q", test.expectedErr.Detail, apiErr.Detail)
}
}
return
} }
if err != nil { if err != nil {
t.Errorf("unexpeceted error %s", err) t.Fatalf("unexpeceted error %s", err)
continue
} }
want, got := test.expected, string(body) want, got := test.expectedBody, string(body)
if want != got { if want != got {
t.Errorf("unexpected body: want %q, got %q", want, got) t.Errorf("unexpected body: want %q, got %q", want, got)
} }
})
} }
} }