Generate client with response#80
Merged
Merged
Conversation
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.
The generated client today exposes only one 2xx response shape per operation - the parser picks one "success" status code (last 2xx in spec order) and treats everything else, including other documented 2xx statuses, as an error. Response headers are dropped on the floor entirely. This is a real problem for any operation that legitimately returns multiple successful statuses with different bodies (e.g. 201 Created with the new resource versus 202 Accepted with a job handle), and for any caller that needs typed access to headers like Location or Retry-After.
There is no straightforward workaround in user code. The classic return type is (*BodyType, error), so additional bodies and headers cannot be reached through the existing API surface; users either fall back to raw *http.Response handling and re-implement decoding, or split the operation into separate spec entries.
What this changes
Introduces an opt-in code-generation flag,
generate.client-with-response, that produces a sibling WithResponse(...) method per operation. The sibling returns a typed envelope whose shape is uniform across operations:mapping the classic generator already uses
The new method handles dispatching on
resp.StatusCodeand decoding into the matching field. Decoding mirrors the classic generator's behavior wherever it overlaps - JSON viajson.Unmarshal, form-urlencoded via the same runtime.ConvertFormFields helper - so the envelope client is a drop-in replacement for any operation the classic client already handles correctly. For text/plain and text/html schemas typed as string, the envelope decodes as a direct string conversion rather than a json.Unmarshal call (which classic does and which fails on plain text bodies); for non-string text schemas it falls through to json.Unmarshal, so integer-valued text/plain count endpoints continue to work.Documented errors (4xx/5xx) populate the envelope's matching status field and return a non-nil error. The error path uses the same runtime.NewClientAPIError and
runtime.WithStatusCodeconstructors the classic client uses, so error semantics are identical - callers can keep their existing if err != nil flow while gaining typed access to the parsed error body when they want it.Backward compatibility
The flag defaults to
false. With it off, generation is byte-for-byte identical to the previous output. No existing user is affected unless they explicitly enable the flag.The two flags interact along an additive matrix. With client: true only, classic methods are emitted exactly as before. With
client-with-response: trueonly, the envelope method is the sole client surface. With both true, the same*Clientstruct gets both methods; the existingClientInterfaceis widened in place to declare the envelope sibling alongside the classic method, so a single mock or test double satisfies both shapes. There is intentionally no secondClientWithResponseInterface. This is a small surface-area change for users implementing their ownClientInterfacefor tests, but those users explicitly opted in by enabling the flag.For specs whose operations do not register any response-location types (e.g. operations that only reference component schemas), the envelope generation seeds the responses output with a header skeleton so the wrapper struct lands in a compilable file regardless of how the underlying spec is structured.
Naming for the new types - the wrapper
<Op>Respand per-status<Op>Resp<Status>Headers- is routed through the shared type tracker, so user-declared schemas with similar names get disambiguated rather than colliding.Extensibility
Two pieces of the new code path are intentionally factored as separate templates so downstream forks that override the client template can reuse them without copying:
Internal data on the operation definition (WithResponseTypeName, HeaderTypeNames, the per-status Successes / Errors slices on the response definition) is the stable contract user templates can depend on.
Why an additive flag rather than a new client style
A mode-style flag (client: classic | envelope) was considered and rejected. Per-call-site choice has real value: many call sites just want the body and the envelope is noise, while others legitimately need headers or multiple 2xx handling. The additive shape lets each call site pick. It also lets the flag default flip later without renaming existing methods.
Why not fix the classic picker
The picker selecting the last spec-order 2xx as "the" success is a latent issue, but it is not what this change addresses. Fixing it would alter generated output for every existing user with multi-2xx operations and is a separable concern; the envelope client makes the picker irrelevant for callers who care about all statuses, which is the primary motivation here.