fix: emit case clauses for bodyless responses to prevent default catch-all unmarshal error#2427
Open
lwc wants to merge 1 commit into
Open
fix: emit case clauses for bodyless responses to prevent default catch-all unmarshal error#2427lwc wants to merge 1 commit into
lwc wants to merge 1 commit into
Conversation
Contributor
Greptile SummaryThis PR updates client response parsing for bodyless OpenAPI responses. The main changes are:
Confidence Score: 3/5This is close, but the wildcard bodyless response ordering should be fixed before merging.
|
| Filename | Overview |
|---|---|
| pkg/codegen/template_helpers.go | Adds no-content response cases, but bodyless wildcard responses can still sort after JSON default catch-all parsing. |
| pkg/codegen/codegen_test.go | Adds regression coverage for exact bodyless responses and explicit status ordering. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pkg/codegen/template_helpers.go:268-269
**Range guard still late**
The no-content range key sorts as `9.nocontent.2XX`, while generated JSON default catch-all keys sort earlier, such as `9.json.default`. For an operation with `2XX` bodyless plus a JSON `default`, a `204` response that includes `Content-Type: application/json` still enters the default JSON unmarshal case before the `2XX` break and returns an EOF error on the empty body. The range guard needs to come after explicit same-range content cases but before the default catch-all.
Reviews (2): Last reviewed commit: "fix: emit case clauses for bodyless resp..." | Re-trigger Greptile
245e50e to
158e810
Compare
Comment on lines
+268
to
+269
| caseKey := fmt.Sprintf("%s.nocontent.%s", prefix, responseName) | ||
| handledCaseClauses[caseKey] = caseClause |
Contributor
There was a problem hiding this comment.
The no-content range key sorts as 9.nocontent.2XX, while generated JSON default catch-all keys sort earlier, such as 9.json.default. For an operation with 2XX bodyless plus a JSON default, a 204 response that includes Content-Type: application/json still enters the default JSON unmarshal case before the 2XX break and returns an EOF error on the empty body. The range guard needs to come after explicit same-range content cases but before the default catch-all.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/template_helpers.go
Line: 268-269
Comment:
**Range guard still late**
The no-content range key sorts as `9.nocontent.2XX`, while generated JSON default catch-all keys sort earlier, such as `9.json.default`. For an operation with `2XX` bodyless plus a JSON `default`, a `204` response that includes `Content-Type: application/json` still enters the default JSON unmarshal case before the `2XX` break and returns an EOF error on the empty body. The range guard needs to come after explicit same-range content cases but before the default catch-all.
How can I resolve this? If you propose a fix, please make it concise.538bb04 to
158e810
Compare
…h-all unmarshal error When a spec defines a response without content (e.g. 204 No Content) alongside a default error response with JSON content, the generated Parse*Response function would attempt to json.Unmarshal an empty body because the default catch-all case (&& true) matched any status code. Bodyless responses produce no type definitions, so the existing loop never visited them and no guarding case clause was emitted. This adds a second pass over all declared responses that emits explicit break clauses for any response without content. Sort key selection ensures correct precedence: - Exact status codes (e.g. 204) sort before content-handling cases, safe because they can only match their own status code. - Range wildcards (e.g. 2XX) sort alongside content-handling cases, preventing a bodyless range from shadowing an explicit response (e.g. 200) that does have content. - default is skipped (a bodyless default is degenerate).
158e810 to
e65caa4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a spec defines a response without content (e.g. 204 No Content) alongside a default error response with JSON content, the generated Parse*Response function would attempt to json.Unmarshal an empty body because the default catch-all case (&& true) matched any status code.
Bodyless responses produce no type definitions, so the existing loop never visited them and no guarding case clause was emitted.
This adds a second pass over all declared responses that emits explicit break clauses for any response without content, placed before the default catch-all in the generated switch statement.
Fixes #2426