Repository navigation
Conversation
Response parsing functions generated by ClientWithResponses silently return the response wrapper when a response's status code and content type match no declared case: the wrapper is left at its zero value and err is nil, so callers cannot distinguish an unrecognised response from a recognised response with an empty body. Add an opt-in output-options.default-response-error that emits a default: case returning an error for such responses. It defaults to false, so existing generated output is unchanged. A spec-declared default response still takes precedence: its case clause is emitted before the generated default: and is evaluated first.
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
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.
Fixes #1923.
Problem
A response parsing function generated by
ClientWithResponsessilently accepts a response that matches no declared case:If the server returns
500and the spec declares only200and404, no case matches. The wrapper is returned witherr == niland its typed fields left at their zero value, so a caller cannot distinguish "unrecognised response" from "recognised response with an empty body". As raised in #1923, that makes an unexpected response easy to miss.Change
New opt-in output option
default-response-error, defaulting tofalseso existing generated output is unchanged. When enabled, the switch gains:The error also propagates through
<Op>WithResponse, which delegates to the parse function.Interaction with a spec-declared
defaultresponseA spec that declares a
defaultresponse still takes precedence: its case clause is emitted before the generateddefault:and is evaluated first. The new case only fires for responses the spec does not describe at all.On the error value
It is a plain
fmt.Errorfcarrying the status code rather than a sentinel, to keep the change small and avoid emitting a new exported symbol into every generated package. If you would prefer an exported sentinel (for exampleErrUnexpectedResponse) that callers can match witherrors.Is, I am happy to switch to that.Tests
New fixture
internal/test/options/default_response_error/, following the existingresponse_getterslayout: one spec declaring200+404, generated twice into separate packages.enabled/: a200still parses normally, and a500returns an error naming the status code.default/: a500still returns the wrapper with no error, pinning the unchanged default behaviour.make tidy && make test && make generate && make lintall pass locally, andmake generateproduces no changes outside the new fixture.AI assistance was used to develop this change. I have reviewed it, tested it, and can explain every line.