Parse response headers into typed fields on client response wrappers#2462
Merged
Conversation
Contributor
Greptile SummaryThis PR adds client-side response header parsing to the generated
Confidence Score: 5/5Safe to merge — the change is purely additive and existing generated output is unaffected when specs have no response headers. The template logic is correct: responsesWithHeaders produces a properly sorted, fresh slice each time it is called; getConditionOfResponseName correctly generates case true: for the OpenAPI default response, which in a tagless Go switch {} only fires when no earlier case matched; and the nullable binding path (headers.Field.Set(value)) is both generated correctly and exercised by a test. No existing identifiers are removed or renamed, the blast radius of the generated diff is proportional to the feature, and all affected fixture files were regenerated. No files require special attention.
|
| Filename | Overview |
|---|---|
| pkg/codegen/templates/client-with-responses.tmpl | Core template change: adds header-struct type definitions and the header-binding switch in ParseResponse; logic is sound and byte-identical for specs with no response headers. |
| pkg/codegen/template_helpers.go | Adds responsesWithHeaders (filter + lexicographic sort) and registers getConditionOfResponseName in TemplateFunctions; sort order correctly puts exact codes before range wildcards before "default". |
| pkg/codegen/operations.go | Adds SchemaType() and SchemaFormat() helpers on ResponseHeaderDefinition for use in the template; both safely handle nil schema and nil type. |
| internal/test/clients/responseheaders/headers_test.go | Tests cover optional/required absent headers, nullable header, component-ref headers, default-response headers, and malformed header error; the nullable path is exercised via nullable-type: true in config.yaml. |
| internal/test/clients/responseheaders/headers.gen.go | Generated client correctly emits three header structs and a three-branch switch (200 → Headers200, 404 → Headers404, true → HeadersDefault); nullable binding via .Set() is correctly generated. |
| internal/test/references/multipackage/pruned_deps/api.gen.go | Correctly adds the runtime import and the 304-headers struct/switch; the new import is the only dependency change and is required by BindStyledParameterWithOptions. |
| internal/test/servers/strict/client/client.gen.go | Strict-server client regenerated with four new header structs and matching switch blocks; nullable-header resolves to *string (not nullable.Nullable) since the client config does not set nullable-type: true, which is correct. |
| internal/test/naming/conflicts/conflicts.gen.go | Correctly regenerated: adds PostFooResponse200Headers and the matching switch in ParsePostFooResponse; no unrelated churn. |
| internal/test/schemas/deprecated/deprecated.gen.go | Regenerated with two new header structs; deprecated header fields correctly emit the Deprecated Go doc comment matching the strict-server convention. |
Reviews (2): Last reviewed commit: "Parse declared response headers into typ..." | Re-trigger Greptile
…wrappers Closes: oapi-codegen#2011 Response headers declared in the spec were dropped by the generated client: the response wrapper exposed only the body fields, leaving declared headers reachable solely through the raw http.Response. The strict server already generates typed header structs and writes them in its Visit* methods; the client had no equivalent on the read side. Each response that declares headers now generates a client-local struct named after the response wrapper (e.g. GetFooResponse200Headers), and the wrapper gains a matching Headers<StatusCode> pointer field (Headers200, HeadersDefault, ...) populated by Parse<Op>Response using runtime.BindStyledParameterWithOptions with ParamLocationHeader ("simple" style, per the OpenAPI spec for response headers). Deriving the type name from the wrapper (via genResponseTypeName) keeps it collision-resolver-aware, and the types are emitted and consumed entirely within the client output, independent of the strict server's <Op><Status>ResponseHeaders types. Binding is lenient: an absent header - even a required one - leaves the field at its zero value rather than failing the parse, mirroring the body unmarshal's tolerance of spec-violating servers; a present header that fails to bind to its declared type is an error. Case clauses are ordered most-specific-first (exact codes, then range wildcards, then default) by the same lexicographic scheme the body unmarshal uses. The change is purely additive: specs without response headers generate byte-identical output, and no strict-server or model output changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mromaszewicz
force-pushed
the
fix/issue-2011
branch
from
July 13, 2026 14:40
81c4116 to
5a67112
Compare
mromaszewicz
added a commit
that referenced
this pull request
Jul 13, 2026
) Closes: #2060 When a response header is an external `$ref` (it lives in another file mapped via import-mapping) and that header's schema is itself a `$ref` to a named type, the generated response-headers struct referenced the type by a bare local name (e.g. `ETag ETagSchema`) instead of the imported one (`ETag externalRef0.ETagSchema`). Because the named type is only generated into the imported package, the referencing package failed to compile with an undefined `ETagSchema`. The header's schema `$ref` is written relative to the external file (e.g. `#/components/schemas/ETagSchema`), so `GenerateGoSchema` resolves it against the root spec as a bare local name. Since the header component carries its own `$ref` telling us which file it came from, qualify the schema with that external package via `ensureExternalRefsInSchema`, mirroring how response content schemas are already handled. This is guarded on the schema being a reference so an external header with an inline primitive schema (e.g. `type: string`) is not mangled into `externalRef0.string`. The fix corrects both the strict-server `<Op><Status>ResponseHeaders` struct and the client response wrapper's header fields introduced in #2462. Adds internal/test/references/multipackage/header_ref exercising an external header ref with a ref'd schema across strict-server and client generation; the committed generated files must compile as part of the test module, which guards against the undefined-symbol regression. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Closes: #2011
Response headers declared in the spec were dropped by the generated client: the response wrapper exposed only the body fields, leaving declared headers reachable solely through the raw http.Response. The strict server already generates typed header structs and writes them in its Visit* methods; the client had no equivalent on the read side.
Each response that declares headers now generates a client-local struct named after the response wrapper (e.g. GetFooResponse200Headers), and the wrapper gains a matching Headers pointer field (Headers200, HeadersDefault, ...) populated by ParseResponse using runtime.BindStyledParameterWithOptions with ParamLocationHeader ("simple" style, per the OpenAPI spec for response headers). Deriving the type name from the wrapper (via genResponseTypeName) keeps it collision-resolver-aware, and the types are emitted and consumed entirely within the client output, independent of the strict server's ResponseHeaders types.
Binding is lenient: an absent header - even a required one - leaves the field at its zero value rather than failing the parse, mirroring the body unmarshal's tolerance of spec-violating servers; a present header that fails to bind to its declared type is an error. Case clauses are ordered most-specific-first (exact codes, then range wildcards, then default) by the same lexicographic scheme the body unmarshal uses.
The change is purely additive: specs without response headers generate byte-identical output, and no strict-server or model output changes.