Fix JSON body conversion for array types in HTTP requests#159
Conversation
Previously, only map[string]any body was converted to JSON string when
Content-Type was application/json. This caused 400 errors when sending
array bodies (e.g., Mackerel API metrics).
Changes:
- Support []any body type for JSON conversion
- Handle all header map types: map[string]any, map[string]string, map[string]interface{}
- Use case-insensitive Content-Type header matching
- Add comprehensive tests for JSON body conversion
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Extract hasJSONContentType() and MarshalBodyIfJSON() from Request() to improve code readability and maintainability. Add unit tests for hasJSONContentType() and MarshalBodyIfJSON(). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Metrics Report
Details | | main (3b12b21) | #159 (e94546d) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 55.3% | 56.0% | +0.6% |
| Files | 65 | 65 | 0 |
| Lines | 6563 | 6578 | +15 |
+ | Covered | 3633 | 3684 | +51 |
+ | Code to Test Ratio | 1:1.0 | 1:1.0 | +0.0 |
| Code | 12892 | 12892 | 0 |
+ | Test | 13267 | 13269 | +2 |
+ | Test Execution Time | 1m22s | 22s | -1m0s |Code coverage of files in pull request scope (53.3% → 82.0%)
Reported by octocov |
Code Metrics Report
Details | | main (3b12b21) | #159 (e94546d) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 55.3% | 56.0% | +0.6% |
| Files | 65 | 65 | 0 |
| Lines | 6563 | 6578 | +15 |
+ | Covered | 3633 | 3685 | +52 |
+ | Code to Test Ratio | 1:1.0 | 1:1.0 | +0.0 |
| Code | 12892 | 12892 | 0 |
+ | Test | 13267 | 13269 | +2 |
+ | Test Execution Time | 1m22s | 22s | -1m0s |Code coverage of files in pull request scope (60.9% → 78.6%)
Reported by octocov |
There was a problem hiding this comment.
Pull request overview
This PR enhances JSON body handling in HTTP requests by supporting array types and improving header type flexibility. Previously, only map[string]any bodies could be converted to JSON; now []any is also supported. The implementation adds case-insensitive Content-Type header matching and handles multiple header map types.
Changes:
- Added support for
[]anybody type in JSON conversion (previously onlymap[string]any) - Implemented case-insensitive Content-Type header matching
- Added handling for multiple header map types:
map[string]any,map[string]string,map[string]interface{}
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| http/client.go | Refactored JSON body conversion into separate functions (hasJSONContentType, MarshalBodyIfJSON) and added support for array bodies and multiple header map types |
| http/client_test.go | Added comprehensive test coverage for JSON body conversion including array bodies, various header map types, and case-insensitive Content-Type matching |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| switch h := headers.(type) { | ||
| case map[string]any: // also matches map[string]interface{} |
There was a problem hiding this comment.
The comment is misleading. In Go, map[string]any and map[string]interface{} are identical at compile time (since any is an alias for interface{}), but the type switch will only match the exact type used in the code. If a caller passes a variable explicitly typed as map[string]interface{}, this case won't match it. The separate handling in the Request function (lines with headersInterfaceMap) confirms they need to be handled as distinct types in practice.
| case map[string]any: // also matches map[string]interface{} | |
| case map[string]any: // headers map with arbitrary value types ('any' is alias of interface{}) |
| "headers": map[string]any{"content-type": "application/json"}, | ||
| "body": `{"already":"json"}`, | ||
| }, | ||
| expectedBody: nil, // not modified |
There was a problem hiding this comment.
The comment 'not modified' is ambiguous. It would be clearer to state 'body should not be set in output map' to match the actual test expectation being verified.
Summary
[]anybody type for JSON conversion (previously onlymap[string]anywas supported)map[string]any,map[string]string,map[string]interface{}content-typeandContent-Typeboth work)Test plan
TestRequestBodyJSONConversion)🤖 Generated with Claude Code