Skip to content

Commit

Permalink
Merge pull request #256 from deploymenttheory/dev-jl
Browse files Browse the repository at this point in the history
Tidy Up Operations
  • Loading branch information
thejoeker12 authored Jul 16, 2024
2 parents 770e157 + 95c6219 commit b9ca2b0
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 10 deletions.
9 changes: 8 additions & 1 deletion httpclient/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func (c *Client) requestWithRetries(method, endpoint string, body, out interface
}

// Success
c.Sugar.Debugf("LOGHERE RETRIES STATUS CODE: %v", resp.StatusCode)
if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusBadRequest {
if resp.StatusCode == http.StatusPermanentRedirect || resp.StatusCode == http.StatusTemporaryRedirect {
c.Sugar.Warn("Redirect response received", zap.Int("status_code", resp.StatusCode), zap.String("location", resp.Header.Get("Location")))
Expand Down Expand Up @@ -217,14 +218,18 @@ func (c *Client) requestWithRetries(method, endpoint string, body, out interface
func (c *Client) requestNoRetries(method, endpoint string, body, out interface{}) (*http.Response, error) {
ctx := context.Background()

c.Sugar.Debug("Executing request without retries", zap.String("method", method), zap.String("endpoint", endpoint))
c.Sugar.Debugw("Executing request without retries", "method", method, "endpoint", endpoint)

resp, err := c.request(ctx, method, endpoint, body)
if err != nil {
return nil, err
}

c.Sugar.Debug("LOGHERE")
c.Sugar.Debugf("Status Code: %v", resp.StatusCode)

if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusBadRequest {
c.Sugar.Debug("FLAG 1 - Request Considered Successful")
if resp.StatusCode == http.StatusPermanentRedirect || resp.StatusCode == http.StatusTemporaryRedirect {
c.Sugar.Warn("Redirect response received", zap.Int("status_code", resp.StatusCode), zap.String("location", resp.Header.Get("Location")))
}
Expand All @@ -233,6 +238,8 @@ func (c *Client) requestNoRetries(method, endpoint string, body, out interface{}
return resp, response.HandleAPISuccessResponse(resp, out, c.Sugar)
}

c.Sugar.Debug("FLAG 2 - Request Considered NOT Successful")

return nil, response.HandleAPIErrorResponse(resp, c.Sugar)
}

Expand Down
2 changes: 1 addition & 1 deletion response/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func HandleAPIErrorResponse(resp *http.Response, sugar *zap.SugaredLogger) *APIE
return apiError
}

mimeType, _ := parseDispositionHeader(resp.Header.Get("Content-Type"))
mimeType, _ := parseHeader(resp.Header.Get("Content-Type"))
switch mimeType {
case "application/json":
parseJSONResponse(bodyBytes, apiError)
Expand Down
2 changes: 1 addition & 1 deletion response/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "strings"
// parseHeader generalizes the parsing of headers like Content-Type and Content-Disposition.
// It extracts the main value (e.g., MIME type for Content-Type) and any parameters (like charset).
// TODO we need to talk about what this does.
func parseDispositionHeader(header string) (string, map[string]string) {
func parseHeader(header string) (string, map[string]string) {
parts := strings.SplitN(header, ";", 2)
mainValue := strings.TrimSpace(parts[0])

Expand Down
15 changes: 10 additions & 5 deletions response/success.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func HandleAPISuccessResponse(resp *http.Response, out interface{}, sugar *zap.S
}

// TODO do we need to redact some auth headers here? I think so.
sugar.Debug("HTTP Response Headers", zap.Any("Headers", resp.Header))
sugar.Debug("Raw HTTP Response", zap.String("Body", string(bodyBytes)))
sugar.Debugw("HTTP Response Headers", zap.Any("Headers", resp.Header))
sugar.Debugw("Raw HTTP Response", zap.String("Body", string(bodyBytes)))

bodyReader := bytes.NewReader(bodyBytes)
contentType := resp.Header.Get("Content-Type")
Expand All @@ -49,7 +49,12 @@ func HandleAPISuccessResponse(resp *http.Response, out interface{}, sugar *zap.S
var handler contentHandler
var ok bool

if handler, ok = responseUnmarshallers[contentType]; ok {
sugar.Debug("LOGHERE-HandleApiSuccessResponse")
sugar.Debugw("Headers:", "content-type", contentType, "content-disposition", contentDisposition)

contentTypeNoParams, _ := parseHeader(contentType)

if handler, ok = responseUnmarshallers[contentTypeNoParams]; ok {
return handler(bodyReader, out, sugar, contentType)
}

Expand All @@ -58,7 +63,7 @@ func HandleAPISuccessResponse(resp *http.Response, out interface{}, sugar *zap.S
}

errMsg := fmt.Sprintf("unexpected MIME type: %s", contentType)
sugar.Error("Unmarshal error", zap.String("content type", contentType), zap.Error(errors.New(errMsg)))
sugar.Errorw("Unmarshal error", zap.String("content type", contentType), zap.Error(errors.New(errMsg)))
return errors.New(errMsg)

}
Expand Down Expand Up @@ -122,7 +127,7 @@ func handleBinaryData(reader io.Reader, sugar *zap.SugaredLogger, out interface{
}

if contentDisposition != "" {
_, params := parseDispositionHeader(contentDisposition)
_, params := parseHeader(contentDisposition)
if filename, ok := params["filename"]; ok {
sugar.Debug("Extracted filename from Content-Disposition", zap.String("filename", filename))
}
Expand Down
4 changes: 2 additions & 2 deletions response/t_parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestParseContentTypeHeader(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotType, gotParams := parseDispositionHeader(tt.header)
gotType, gotParams := parseHeader(tt.header)
if gotType != tt.wantType {
t.Errorf("ParseContentTypeHeader() gotType = %v, want %v", gotType, tt.wantType)
}
Expand Down Expand Up @@ -70,7 +70,7 @@ func TestParseContentDisposition(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotType, gotParams := parseDispositionHeader(tt.header)
gotType, gotParams := parseHeader(tt.header)
if gotType != tt.wantType {
t.Errorf("ParseContentDisposition() gotType = %v, want %v", gotType, tt.wantType)
}
Expand Down

0 comments on commit b9ca2b0

Please sign in to comment.