feat: add client-response-error-on-unexpected-response output option - #2540
feat: add client-response-error-on-unexpected-response output option#2540ChrisJr404 wants to merge 1 commit into
Conversation
Sorry, something went wrong.
When enabled, the generated Parse<Operation>Response functions get a default case that returns a new ErrUnexpectedResponse sentinel when the response status code and content-type match none of the responses declared in the spec. Off by default, so existing output is unchanged.
Greptile SummaryThis PR adds an opt-in output option that makes generated response parsers return a shared sentinel for undeclared status/content-type combinations.
Confidence Score: 3/5The PR should not merge until response parsers honor the option for operations without decoder clauses and the new sentinel cannot collide with generated declarations. Valid specifications can either produce uncompilable Go through an ErrUnexpectedResponse name collision or silently retain the old nil-error behavior because response generation exits before adding the configured default branch. Files Needing Attention: pkg/codegen/template_helpers.go, pkg/codegen/templates/client-with-responses.tmpl Important Files Changed
### Issue 1
pkg/codegen/templates/client-with-responses.tmpl:22
**Sentinel collides with schema names**
When the option is enabled for a specification containing a component whose generated name is `ErrUnexpectedResponse`, this fixed package-level variable duplicates the generated type identifier, causing the generated Go package to fail compilation with a redeclaration error.
### Issue 2
pkg/codegen/template_helpers.go:281-283
**Empty decoders bypass unexpected errors**
When the option is enabled for an operation whose responses produce no decoder clauses, `genResponseUnmarshal` returns before emitting this default branch, so an undeclared status still returns a response with a nil error instead of `ErrUnexpectedResponse`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat: add client-response-error-on-unexp..." | Re-trigger Greptile |
Sorry, something went wrong.
| // ErrUnexpectedResponse is returned by the Parse<Operation>Response functions | ||
| // when the server responds with a status code and content-type that match none | ||
| // of the responses declared in the OpenAPI specification. | ||
| var ErrUnexpectedResponse = errors.New("unexpected response") |
There was a problem hiding this comment.
Sentinel collides with schema names
When the option is enabled for a specification containing a component whose generated name is ErrUnexpectedResponse, this fixed package-level variable duplicates the generated type identifier, causing the generated Go package to fail compilation with a redeclaration error.
Knowledge Base Used: Template rendering system
Prompt To Fix With AIThis is a comment left during a code review.
Path: pkg/codegen/templates/client-with-responses.tmpl
Line: 22
Comment:
**Sentinel collides with schema names**
When the option is enabled for a specification containing a component whose generated name is `ErrUnexpectedResponse`, this fixed package-level variable duplicates the generated type identifier, causing the generated Go package to fail compilation with a redeclaration error.
**Knowledge Base Used:** [Template rendering system](https://app.greptile.com/oapi-codegen/-/custom-context/knowledge-base/oapi-codegen/oapi-codegen/-/docs/template-rendering.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Sorry, something went wrong.
| if globalState.options.OutputOptions.ClientResponseErrorOnUnexpectedResponse { | ||
| fmt.Fprintf(buffer, "default:\nreturn nil, ErrUnexpectedResponse\n") | ||
| } |
There was a problem hiding this comment.
Empty decoders bypass unexpected errors
When the option is enabled for an operation whose responses produce no decoder clauses, genResponseUnmarshal returns before emitting this default branch, so an undeclared status still returns a response with a nil error instead of ErrUnexpectedResponse.
Knowledge Base Used: Operation and client generation
Prompt To Fix With AIThis is a comment left during a code review.
Path: pkg/codegen/template_helpers.go
Line: 281-283
Comment:
**Empty decoders bypass unexpected errors**
When the option is enabled for an operation whose responses produce no decoder clauses, `genResponseUnmarshal` returns before emitting this default branch, so an undeclared status still returns a response with a nil error instead of `ErrUnexpectedResponse`.
**Knowledge Base Used:** [Operation and client generation](https://app.greptile.com/oapi-codegen/-/custom-context/knowledge-base/oapi-codegen/oapi-codegen/-/docs/operation-and-client-generation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
Summary
Closes #1923.
When you only declare a couple of the responses you care about, the generated Parse<Operation>Response today silently returns a response object with just Body/HTTPResponse set (and a nil error) for anything the spec didn't mention, so there's no easy way to notice an unexpected response.
This adds an opt-in output-options flag, client-response-error-on-unexpected-response, that appends a default case to the response switch returning a new ErrUnexpectedResponse sentinel:
var ErrUnexpectedResponse = errors.New("unexpected response") is emitted alongside the client, so callers can errors.Is(err, ErrUnexpectedResponse).
It's off by default, so existing generated code is unchanged (confirmed by re-running make generate with no diffs elsewhere). New internal/test/options/unexpected_response covers both the enabled path (undeclared status returns the sentinel, a declared 200 still parses fine) and the default path (undeclared status still returns a response with a nil error).
Docs and configuration-schema.json updated to match.