From 72f68d3c03e0ca2c24c7ecc700bb67d267b2f013 Mon Sep 17 00:00:00 2001 From: Nathan Date: Wed, 26 May 2021 22:40:00 +0000 Subject: [PATCH 1/2] Adjust HTTPError to read response body before it is closed --- coder-sdk/activity.go | 2 +- coder-sdk/error.go | 40 +++++++++++++++++++++++++++++++--------- coder-sdk/request.go | 2 +- coder-sdk/ws.go | 2 +- internal/cmd/auth.go | 2 +- internal/cmd/errors.go | 2 +- wsnet/dial.go | 4 +--- wsnet/listen.go | 4 +--- 8 files changed, 38 insertions(+), 20 deletions(-) diff --git a/coder-sdk/activity.go b/coder-sdk/activity.go index 6d564efb..23c75abd 100644 --- a/coder-sdk/activity.go +++ b/coder-sdk/activity.go @@ -21,7 +21,7 @@ func (c *DefaultClient) PushActivity(ctx context.Context, source, workspaceID st } if resp.StatusCode != http.StatusOK { - return bodyError(resp) + return NewHTTPError(resp) } return nil } diff --git a/coder-sdk/error.go b/coder-sdk/error.go index 2973046a..56446897 100644 --- a/coder-sdk/error.go +++ b/coder-sdk/error.go @@ -1,8 +1,10 @@ package coder import ( + "bytes" "encoding/json" "fmt" + "io" "net/http" "golang.org/x/xerrors" @@ -29,11 +31,31 @@ type APIErrorMsg struct { Details json.RawMessage `json:"details"` } +// NewHTTPError reads the response body and stores metadata +// about the response in order to be unpacked into +// an *APIError. +func NewHTTPError(resp *http.Response) *HTTPError { + var buf bytes.Buffer + _, err := io.CopyN(&buf, resp.Body, 1<<20) + if err != nil { + return &HTTPError{ + cachedErr: err, + } + } + return &HTTPError{ + url: resp.Request.URL.String(), + statusCode: resp.StatusCode, + body: buf.Bytes(), + } +} + // HTTPError represents an error from the Coder API. type HTTPError struct { - *http.Response - cached *APIError - cachedErr error + url string + statusCode int + body []byte + cached *APIError + cachedErr error } // Payload decode the response body into the standard error structure. The `details` @@ -46,7 +68,7 @@ func (e *HTTPError) Payload() (*APIError, error) { // Try to decode the payload as an error, if it fails or if there is no error message, // return the response URL with the status. - if err := json.NewDecoder(e.Response.Body).Decode(&msg); err != nil { + if err := json.Unmarshal(e.body, &msg); err != nil { e.cachedErr = err return nil, err } @@ -55,16 +77,16 @@ func (e *HTTPError) Payload() (*APIError, error) { return &msg, nil } +func (e *HTTPError) StatusCode() int { + return e.statusCode +} + func (e *HTTPError) Error() string { apiErr, err := e.Payload() if err != nil || apiErr.Err.Msg == "" { - return fmt.Sprintf("%s: %d %s", e.Request.URL, e.StatusCode, e.Status) + return fmt.Sprintf("%s: %d %s", e.url, e.statusCode, http.StatusText(e.statusCode)) } // If the payload was a in the expected error format with a message, include it. return apiErr.Err.Msg } - -func bodyError(resp *http.Response) error { - return &HTTPError{Response: resp} -} diff --git a/coder-sdk/request.go b/coder-sdk/request.go index a49ddda1..d8f8bb76 100644 --- a/coder-sdk/request.go +++ b/coder-sdk/request.go @@ -114,7 +114,7 @@ func (c *DefaultClient) requestBody(ctx context.Context, method, path string, in // Responses in the 100 are handled by the http lib, in the 200 range, we have a success. // Consider anything at or above 300 to be an error. if resp.StatusCode > 299 { - return fmt.Errorf("unexpected status code %d: %w", resp.StatusCode, bodyError(resp)) + return fmt.Errorf("unexpected status code %d: %w", resp.StatusCode, NewHTTPError(resp)) } // If we expect a payload, process it as json. diff --git a/coder-sdk/ws.go b/coder-sdk/ws.go index 6f6a920f..89cb28e8 100644 --- a/coder-sdk/ws.go +++ b/coder-sdk/ws.go @@ -26,7 +26,7 @@ func (c *DefaultClient) dialWebsocket(ctx context.Context, path string, options conn, resp, err := websocket.Dial(ctx, url.String(), &websocket.DialOptions{HTTPHeader: headers}) if err != nil { if resp != nil { - return nil, bodyError(resp) + return nil, NewHTTPError(resp) } return nil, err } diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 56aaef80..2c59611a 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -66,7 +66,7 @@ func newClient(ctx context.Context, checkVersion bool) (coder.Client, error) { if err != nil { var he *coder.HTTPError if xerrors.As(err, &he) { - if he.StatusCode == http.StatusUnauthorized { + if he.StatusCode() == http.StatusUnauthorized { return nil, xerrors.Errorf("not authenticated: try running \"coder login`\"") } } diff --git a/internal/cmd/errors.go b/internal/cmd/errors.go index e202037e..dce13918 100644 --- a/internal/cmd/errors.go +++ b/internal/cmd/errors.go @@ -61,7 +61,7 @@ func handleAPIError(origError error) error { return origError } - return clog.Error(fmt.Sprintf("Precondition Error : Status Code=%d", httpError.StatusCode), + return clog.Error(fmt.Sprintf("Precondition Error : Status Code=%d", httpError.StatusCode()), p.Message, clog.BlankLine, clog.Tipf(p.Solution)) diff --git a/wsnet/dial.go b/wsnet/dial.go index c8f0f7e3..6900bacf 100644 --- a/wsnet/dial.go +++ b/wsnet/dial.go @@ -24,9 +24,7 @@ func DialWebsocket(ctx context.Context, broker string, iceServers []webrtc.ICESe defer func() { _ = resp.Body.Close() }() - return nil, &coder.HTTPError{ - Response: resp, - } + return nil, coder.NewHTTPError(resp) } return nil, fmt.Errorf("dial websocket: %w", err) } diff --git a/wsnet/listen.go b/wsnet/listen.go index c11df79c..92f7671f 100644 --- a/wsnet/listen.go +++ b/wsnet/listen.go @@ -81,9 +81,7 @@ func (l *listener) dial(ctx context.Context) (<-chan error, error) { conn, resp, err := websocket.Dial(ctx, l.broker, nil) if err != nil { if resp != nil { - return nil, &coder.HTTPError{ - Response: resp, - } + return nil, coder.NewHTTPError(resp) } return nil, err } From a7befe2670cf42b85a626763302e61328407312f Mon Sep 17 00:00:00 2001 From: Nathan Date: Wed, 26 May 2021 23:05:33 +0000 Subject: [PATCH 2/2] Remove response size limitation --- coder-sdk/error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coder-sdk/error.go b/coder-sdk/error.go index 56446897..9e1645d0 100644 --- a/coder-sdk/error.go +++ b/coder-sdk/error.go @@ -36,7 +36,7 @@ type APIErrorMsg struct { // an *APIError. func NewHTTPError(resp *http.Response) *HTTPError { var buf bytes.Buffer - _, err := io.CopyN(&buf, resp.Body, 1<<20) + _, err := io.Copy(&buf, resp.Body) if err != nil { return &HTTPError{ cachedErr: err,