Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Commit 2d470e4

Browse files
author
Nathan Potter
authored
Adjust HTTPError to read response body before it is closed (#361)
* Adjust HTTPError to read response body before it is closed * Remove response size limitation
1 parent 0d648e8 commit 2d470e4

File tree

8 files changed

+38
-20
lines changed

8 files changed

+38
-20
lines changed

‎coder-sdk/activity.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func (c *DefaultClient) PushActivity(ctx context.Context, source, workspaceID st
2121
}
2222

2323
if resp.StatusCode != http.StatusOK {
24-
return bodyError(resp)
24+
return NewHTTPError(resp)
2525
}
2626
return nil
2727
}

‎coder-sdk/error.go

+31-9
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package coder
22

33
import (
4+
"bytes"
45
"encoding/json"
56
"fmt"
7+
"io"
68
"net/http"
79

810
"golang.org/x/xerrors"
@@ -29,11 +31,31 @@ type APIErrorMsg struct {
2931
Details json.RawMessage `json:"details"`
3032
}
3133

34+
// NewHTTPError reads the response body and stores metadata
35+
// about the response in order to be unpacked into
36+
// an *APIError.
37+
func NewHTTPError(resp *http.Response) *HTTPError {
38+
var buf bytes.Buffer
39+
_, err := io.Copy(&buf, resp.Body)
40+
if err != nil {
41+
return &HTTPError{
42+
cachedErr: err,
43+
}
44+
}
45+
return &HTTPError{
46+
url: resp.Request.URL.String(),
47+
statusCode: resp.StatusCode,
48+
body: buf.Bytes(),
49+
}
50+
}
51+
3252
// HTTPError represents an error from the Coder API.
3353
type HTTPError struct {
34-
*http.Response
35-
cached *APIError
36-
cachedErr error
54+
url string
55+
statusCode int
56+
body []byte
57+
cached *APIError
58+
cachedErr error
3759
}
3860

3961
// Payload decode the response body into the standard error structure. The `details`
@@ -46,7 +68,7 @@ func (e *HTTPError) Payload() (*APIError, error) {
4668

4769
// Try to decode the payload as an error, if it fails or if there is no error message,
4870
// return the response URL with the status.
49-
if err := json.NewDecoder(e.Response.Body).Decode(&msg); err != nil {
71+
if err := json.Unmarshal(e.body, &msg); err != nil {
5072
e.cachedErr = err
5173
return nil, err
5274
}
@@ -55,16 +77,16 @@ func (e *HTTPError) Payload() (*APIError, error) {
5577
return &msg, nil
5678
}
5779

80+
func (e *HTTPError) StatusCode() int {
81+
return e.statusCode
82+
}
83+
5884
func (e *HTTPError) Error() string {
5985
apiErr, err := e.Payload()
6086
if err != nil || apiErr.Err.Msg == "" {
61-
return fmt.Sprintf("%s: %d %s", e.Request.URL, e.StatusCode, e.Status)
87+
return fmt.Sprintf("%s: %d %s", e.url, e.statusCode, http.StatusText(e.statusCode))
6288
}
6389

6490
// If the payload was a in the expected error format with a message, include it.
6591
return apiErr.Err.Msg
6692
}
67-
68-
func bodyError(resp *http.Response) error {
69-
return &HTTPError{Response: resp}
70-
}

‎coder-sdk/request.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (c *DefaultClient) requestBody(ctx context.Context, method, path string, in
114114
// Responses in the 100 are handled by the http lib, in the 200 range, we have a success.
115115
// Consider anything at or above 300 to be an error.
116116
if resp.StatusCode > 299 {
117-
return fmt.Errorf("unexpected status code %d: %w", resp.StatusCode, bodyError(resp))
117+
return fmt.Errorf("unexpected status code %d: %w", resp.StatusCode, NewHTTPError(resp))
118118
}
119119

120120
// If we expect a payload, process it as json.

‎coder-sdk/ws.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func (c *DefaultClient) dialWebsocket(ctx context.Context, path string, options
2626
conn, resp, err := websocket.Dial(ctx, url.String(), &websocket.DialOptions{HTTPHeader: headers})
2727
if err != nil {
2828
if resp != nil {
29-
return nil, bodyError(resp)
29+
return nil, NewHTTPError(resp)
3030
}
3131
return nil, err
3232
}

‎internal/cmd/auth.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func newClient(ctx context.Context, checkVersion bool) (coder.Client, error) {
6666
if err != nil {
6767
var he *coder.HTTPError
6868
if xerrors.As(err, &he) {
69-
if he.StatusCode == http.StatusUnauthorized {
69+
if he.StatusCode() == http.StatusUnauthorized {
7070
return nil, xerrors.Errorf("not authenticated: try running \"coder login`\"")
7171
}
7272
}

‎internal/cmd/errors.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func handleAPIError(origError error) error {
6161
return origError
6262
}
6363

64-
return clog.Error(fmt.Sprintf("Precondition Error : Status Code=%d", httpError.StatusCode),
64+
return clog.Error(fmt.Sprintf("Precondition Error : Status Code=%d", httpError.StatusCode()),
6565
p.Message,
6666
clog.BlankLine,
6767
clog.Tipf(p.Solution))

‎wsnet/dial.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ func DialWebsocket(ctx context.Context, broker string, iceServers []webrtc.ICESe
2424
defer func() {
2525
_ = resp.Body.Close()
2626
}()
27-
return nil, &coder.HTTPError{
28-
Response: resp,
29-
}
27+
return nil, coder.NewHTTPError(resp)
3028
}
3129
return nil, fmt.Errorf("dial websocket: %w", err)
3230
}

‎wsnet/listen.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,7 @@ func (l *listener) dial(ctx context.Context) (<-chan error, error) {
8181
conn, resp, err := websocket.Dial(ctx, l.broker, nil)
8282
if err != nil {
8383
if resp != nil {
84-
return nil, &coder.HTTPError{
85-
Response: resp,
86-
}
84+
return nil, coder.NewHTTPError(resp)
8785
}
8886
return nil, err
8987
}

0 commit comments

Comments
 (0)