Skip to content

Commit

Permalink
(fix): improve status handling in API Domain
Browse files Browse the repository at this point in the history
Properly handle cases where a non-200 status code is returned and the status text is in the response body - otherwise basically any error would end up in the body as a string (thanks, http/2) and result in an unmarshalling error.

Changes:
- added a "status" field, which is populated with the http.Status if included in the response, or the text from the http.StatusCode if not.
- don't error on a 404 (I swear I did that already, so I guess I did it for reals this time)
- don't try to unmarshal the body into a json object if the return isn't a 2XX

I'm sure I'm missing more edge cases, and welcome any suggestions, but hopefully this is better!
  • Loading branch information
mildwonkey committed Jan 8, 2025
1 parent 3219c19 commit 250bf50
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 21 deletions.
17 changes: 10 additions & 7 deletions src/pkg/domains/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import (
)

type APIResponse struct {
Status int
Raw json.RawMessage
Response any
StatusCode int
Status string
Raw json.RawMessage
Response any
}

func (a ApiDomain) makeRequests(ctx context.Context) (types.DomainResources, error) {
Expand Down Expand Up @@ -63,11 +64,13 @@ func (a ApiDomain) makeRequests(ctx context.Context) (types.DomainResources, err
errs = errors.Join(errs, err)
}
if response != nil {
collection[request.Name] = types.DomainResources{
"status": response.Status,
"raw": response.Raw,
"response": response.Response,
dr := types.DomainResources{
"status": response.Status,
"statuscode": response.StatusCode,
"raw": response.Raw,
"response": response.Response,
}
collection[request.Name] = dr
} else {
// If the entire response is empty, return a validly empty resource
collection[request.Name] = types.DomainResources{"status": 0}
Expand Down
21 changes: 15 additions & 6 deletions src/pkg/domains/api/http_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,26 @@ func doHTTPReq(ctx context.Context, client http.Client, method string, url url.U
return nil, fmt.Errorf("error: calling %s returned empty response", url.Redacted())
}
defer res.Body.Close()

var respObj APIResponse
respObj.StatusCode = res.StatusCode
if res.Status == "" {
respObj.Status = http.StatusText(res.StatusCode)
} else {
respObj.Status = res.Status
}
responseData, err := io.ReadAll(res.Body)
if err != nil {
return nil, err
}

var respObj APIResponse
respObj.Raw = responseData
respObj.Status = res.StatusCode
err = json.Unmarshal(responseData, &respObj.Response)
return &respObj, err
if respObj.StatusCode >= http.StatusOK && respObj.StatusCode < http.StatusMultiStatus {
respObj.Raw = responseData
err = json.Unmarshal(responseData, &respObj.Response)
if err != nil {
return nil, err
}
}
return &respObj, nil
}

func clientFromOpts(opts *ApiOpts) http.Client {
Expand Down
19 changes: 11 additions & 8 deletions src/pkg/domains/api/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ func TestGetResources(t *testing.T) {
"response": map[string]interface{}{
"healthcheck": "ok",
},
"status": 200,
"status": "200 OK",
"statuscode": 200,
}}
require.Equal(t, want, drs)
})
Expand All @@ -135,13 +136,15 @@ func TestGetResources(t *testing.T) {

require.NoError(t, err) // the spec is correct
drs, err := api.GetResources(context.Background())
require.Error(t, err)
require.Equal(t, drs, types.DomainResources{
require.NoError(t, err)
require.Equal(t, types.DomainResources{
apiReqName: types.DomainResources{
"status": 400,
"raw": json.RawMessage{},
"response": nil,
},
})
"statuscode": 400,
"status": "400 Bad Request",
"response": nil,
"raw": json.RawMessage(nil),
}},
drs,
)
})
}

0 comments on commit 250bf50

Please sign in to comment.