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
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
|
| Filename | Overview |
|---|---|
| pkg/codegen/template_helpers.go | Adds the unexpected-response default branch, but existing early returns prevent it from being generated for operations without decoder clauses. |
| pkg/codegen/templates/client-with-responses.tmpl | Adds the sentinel declaration, but its fixed package-level identifier can collide with generated schema declarations. |
| pkg/codegen/configuration.go | Adds the opt-in output option with a YAML key and documented default behavior. |
| configuration-schema.json | Keeps the JSON configuration schema synchronized with the new output option. |
| internal/test/options/unexpected_response/enabled/unexpected_response_test.go | Covers a declared JSON response and an undeclared status, but not empty-decoder operations or generated-name collisions. |
Prompt To Fix All With AI
### 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
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // 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.
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 AI
This 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.| if globalState.options.OutputOptions.ClientResponseErrorOnUnexpectedResponse { | ||
| fmt.Fprintf(buffer, "default:\nreturn nil, ErrUnexpectedResponse\n") | ||
| } |
There was a problem hiding this comment.
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 AI
This 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.1 participant
Summary
Closes #1923.
When you only declare a couple of the responses you care about, the generated
Parse<Operation>Responsetoday silently returns a response object with justBody/HTTPResponseset (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-optionsflag,client-response-error-on-unexpected-response, that appends adefaultcase to the response switch returning a newErrUnexpectedResponsesentinel:var ErrUnexpectedResponse = errors.New("unexpected response")is emitted alongside the client, so callers canerrors.Is(err, ErrUnexpectedResponse).It's off by default, so existing generated code is unchanged (confirmed by re-running
make generatewith no diffs elsewhere). Newinternal/test/options/unexpected_responsecovers 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.jsonupdated to match.