feat(http): add bodytext field to response object to always return response body as string #5106
Conversation
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…sponse body as string Signed-off-by: Yiran Cao <yiran0427@gmail.com>
yiran0427
force-pushed
the
yiran/5105
branch
from
75db128 to
5fbc50f
Compare
krancour
added
priority/low
| // Log the JSON parsing error but continue to provide bodyText | ||
| logging.LoggerFromContext(ctx).Debug( | ||
| "Failed to parse JSON response, continuing with bodyText only", | ||
| "error", err, | ||
| ) | ||
| } else { | ||
| // Unmarshal into map[string]any or []any | ||
| switch parsedBody.(type) { | ||
| case map[string]any, []any: | ||
| env["response"].(map[string]any)["body"] = parsedBody // nolint: forcetypeassert | ||
| default: | ||
| logging.LoggerFromContext(ctx).Debug( | ||
| "Unexpected JSON type, continuing with bodyText only", | ||
| "type", fmt.Sprintf("%T", parsedBody), | ||
| ) | ||
| } |
There was a problem hiding this comment.
This doesn't feel right. To have gotten to this chunk of code, the conditional on line +300 evaluated to true. The content type of the response was explicitly JSON or the response has already been determined to be valid JSON (regardless of what the content type said). i.e. If we're in this block of code, it's expected that the body will unmarshal correctly. If it doesn't we should be sticking with the original behavior of returning an error.
There was a problem hiding this comment.
oh i see. is it better if i add another case for contentType == 'application/json'. then return bodyText: string(bodyBytes) and empty body?
There was a problem hiding this comment.
I don't think you need to do anything except revert the changes to these lines.
I think the only change needed to this file is the one on line +296.
krancour
added
the
hacktoberfest
2 participants
Fixes #5105