Declare non-JSON response content types in the typespec - #23
Conversation
A response body was always documented and sent as application/json, so an
action serving PDF, XML or any other bytes had no way to describe itself: the
generated spec claimed a JSON string and a generated client would send
Accept: application/json.
A `content-type` entry in a response headers map, with the media type as a
literal atom, is now that response's media type declaration:
@SPEC mandate_pdf(Plug.Conn.t(), %{id: String.t()}, %{}, %{}, nil) ::
{200, %{"content-type": :"application/pdf"}, binary()}
| {404, %{}, Error.t()}
The OpenAPI generator passes it to Spectral.OpenAPI.response_with_body/4 so the
body is emitted under that media type, and leaves the entry out of the response
headers object. At runtime a non-JSON body is sent verbatim (it must be a
binary) with the declared content type, so an action no longer has to reach for
conn to serve raw bytes; application/json and *+json bodies are still encoded by
Spectral. The declaration is per response, so other statuses in the same union
stay JSON, and it documents actions that return conn for streaming or file sends.
The example app gains a text/vcard download endpoint covering the whole path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
There was a problem hiding this comment.
🟡 Changes recommended
A moderate runtime issue remains where nil can bypass the declared non-JSON media-type contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds typespec-driven response media types for non-JSON bodies, updating OpenAPI generation and runtime handling.
Changes:
- Supports literal-atom
content-typedeclarations per response. - Sends non-JSON binary bodies verbatim while preserving JSON behavior.
- Adds documentation, tests, changelog updates, and a vCard example endpoint.
File summaries
| File | Summary | Review notes |
|---|---|---|
test/support/test_content_type_router.ex |
Adds content-type test routes. | — |
test/support/test_content_type_controller.ex |
Adds content-type test actions. | — |
test/phoenix_spectral/controller_test.exs |
Tests runtime response behavior. | — |
test/phoenix_spectral_test.exs |
Tests generated OpenAPI media types. | — |
README.md |
Documents non-JSON responses and streaming usage. | Nit (1 vote): update the streaming example’s response header type and payload type. |
lib/phoenix_spectral/internal.ex |
Extracts and validates declared media types. | Nit (1 vote): add regression coverage for non-literal content-type values. |
lib/phoenix_spectral/controller.ex |
Handles raw and JSON response bodies. | Moderate (3 votes): the nil fast path bypasses the non-JSON media-type check; this also affects line 438. |
lib/phoenix_spectral.ex |
Routes declared media types into OpenAPI responses. | — |
example/test/example_test.exs |
Adds vCard integration tests. | — |
example/lib/example/user_controller.ex |
Adds the vCard download endpoint. | — |
example/lib/example/router.ex |
Registers the vCard route. | — |
CLAUDE.md |
Updates example-app guidance. | — |
CHANGELOG.md |
Records the feature. | — |
Review details
Suppressed comments (3)
README.md:207
- The streaming example immediately above still declares
{200, %{}, nil}while it sendsapplication/octet-stream. With the new generator behavior, that endpoint is documented asapplication/json(and anilbody type cannot describe the streamed bytes), contradicting this guidance and leaving the example's generated spec wrong. Update the example's response header type and payload type to describe the bytes it sends.
**When a conn is returned, PhoenixSpectral passes it through without schema validation.** The typespec still documents the endpoint for the OpenAPI spec, but the actual response is your responsibility — including its `content-type`, which belongs in the typespec too (see [Non-JSON response bodies](#non-json-response-bodies)) so the generated spec matches what the action sends.
lib/phoenix_spectral/controller.ex:440
- Media-type tokens are case-insensitive, but this comparison is not. A valid declaration such as
:"Application/JSON"or:"Application/Problem+JSON"is treated as non-JSON, so a struct body is sent through the raw-binary path and raises instead of being encoded as promised forapplication/jsonand*+json. Normalize the value before comparing the type and structured suffix.
defp json_content_type?(content_type) do
content_type == @default_content_type or String.ends_with?(content_type, "+json")
end
lib/phoenix_spectral/internal.ex:76
- Please add a generator regression test for the documented invalid shape where
content-typeis not a literal atom. The newArgumentErrorbranch is currently untested, so a future change could turn this into an opaque pattern-match failure or accidentally accept a dynamic type without any test detecting it.
defp content_type(literal_map_field(val_type: sp_literal(binary_value: content_type))) do
content_type
end
defp content_type(literal_map_field(val_type: val_type)) do
raise ArgumentError,
"PhoenixSpectral: the \"content-type\" entry of a response headers map declares the " <>
"response media type and must be a literal atom, e.g. " <>
~s(%{"content-type": :"application/pdf"}, got: #{inspect(val_type)})
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Media type tokens are case-insensitive, so a response declaring :"Application/JSON" or :"Application/Problem+JSON" took the raw-bytes path and raised on a struct body instead of being encoded as JSON. Downcase before comparing, matching how the content-type header name is already matched. Also cover the two untested edges Copilot flagged — a content-type entry that is not a literal atom, and a nil body type under a declared media type — and fix the README streaming example, whose nil body type would now document the endpoint as an empty application/json response rather than the bytes it sends. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
A review pass over the feature turned up four real holes, all in how a declared media type is validated and compared: - A media type carrying parameters was compared whole, so :"application/json; charset=utf-8" generated a plausible spec and then raised on the first request. Only the type and subtype decide whether a body is JSON now, and a declared charset is no longer duplicated on the header. - Declaring a non-JSON media type over a struct body generated a JSON schema under that media type, which the endpoint could never serve. It raises while the spec is generated instead, matching what the runtime requires. - Two content-type entries (:"content-type" and :"Content-Type" are distinct atoms) crashed with a raw spectra record dump rather than an explanation. - An entry whose value was a literal without a "/" in it, such as an integer, slipped past the literal-atom guard and was sent as the media type. The header-name casing and *+json branches had no test at all: deleting either left both suites green. They, and each case above, now have one that fails without the fix. The charset asymmetry between the two send paths also collapses into a single call. The declaration is a breaking change for a response headers map that declared a dynamic content-type header, which 0.6.x documented and then overwrote; the changelog now says so under Changed. The example app's vCard endpoint drops the key it was only constructing to discard, declares the charset RFC 6350 requires, and emits CRLF line endings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
The moduledoc example declared `%{"content-type": :"application/pdf"}` as a
required key and dutifully returned it, then the paragraph below said the media
type is read from the typespec and the entry is not a response header — which
reads as a contradiction without the required-vs-optional detail that only the
README carried.
Both examples now declare `optional(:"content-type")` and return `%{}`, matching
the example app's vCard endpoint, and the prose says why: `optional/1` is what
lets the action leave the entry out, the shorthand makes Dialyzer expect it, and
the returned value is ignored either way.
Docs only; no behaviour change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
The runtime popped the `content-type` field out of the response headers map before validating the returned headers, so the value an action returned there was silently ignored: a required declaration could be omitted with no error, and returning `:"application/xml"` under a `:"application/pdf"` declaration went unnoticed. That is the one response header spectra did not check against the typespec, which is also what made the docs read as a contradiction — the example returned an entry the prose said was never read. The field now stays in the set passed to `encode_response_headers/5`, so it goes through the same `Spectral.encode` check and the same required-key error as the headers around it. What it still does not do is travel the ordinary header path: the response's `content-type` is set from the declared media type (keeping the `; charset=utf-8` rule for JSON), not from the returned value, and a declaration spelled `Content-Type` therefore cannot reach `put_resp_header/3`, which rejects non-lowercase names. Both examples go back to the required form, returning the entry, and the docs say what is validated, what is sent, and what `optional/1` buys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
|
Re the
Yes on both counts, and the second half wasn't true of the code either. Fixed in 56e8715. In the API response it always was: Returned from the code it now is, validated. The runtime popped the What it still does not do is travel the ordinary header path: the response's Both examples go back to the required form, returning the entry: @spec mandate_pdf(Plug.Conn.t(), %{id: String.t()}, %{}, %{}, nil) ::
{200, %{"content-type": :"application/pdf"}, binary()}
| {404, %{}, Error.t()}
def mandate_pdf(_conn, %{id: id}, _query_params, _headers, _body) do
case Documents.pdf(id) do
{:ok, pdf} -> {200, %{"content-type": :"application/pdf"}, pdf}
:not_found -> {404, %{}, %Error{message: "Not found"}}
end
end
Tests: Generated by Claude Code |
`map_fields/2` and `binary_body_type?/2` each carried the same two-clause
resolution of a user type reference and a remote one. Both are now
`resolve_type_ref/2`, which returns the resolved type together with the type
info it belongs to — the pair has to travel, since a remote reference resolves
in its own module's type info.
spectra has a `resolve_type_ref/2` of its own (spectra.erl), but it is not
exported and only covers local references; remote ones it resolves per format
module, with codec and meta handling we deliberately do not replicate. The one
piece worth borrowing is `spectra_util:apply_args/3`, which substitutes a
reference's arguments for the alias's variables.
That fixes a bug the duplication hid: both sites looked a user type reference up
at arity 0, so a parameterized alias in a response headers map or body died on
the `{:ok, _} =` match. The reference carries its own arity, and now that is
what is used.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
There was a problem hiding this comment.
🔵 Needs a closer look
Runtime handling must reject binary returns when the declared body type is nil, with a regression test.
Review details
Suppressed comments (1)
lib/phoenix_spectral/controller.ex:447
binary_body_type?/2intentionally accepts thenilbody type, but the runtime raw path only checks the returned value. Thus a spec such as{200, %{optional(:"content-type") => :"application/pdf"}, nil}can return a binary, fall through toraw_response_body/2, and send bytes even thoughresponse_with_body/4generated no response content. Add a guard forsp_literal(value: nil)that rejects any non-nilreturn, while retaining the empty-body shortcut for anilreturn, and cover this mismatch with a regression test.
defp raw_response_body(body, _content_type) when is_binary(body), do: body
defp raw_response_body(body, content_type) do
raise "PhoenixSpectral: a response declaring content type #{content_type} must return a " <>
"binary body, got: #{inspect(body)}"
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
`binary_body_type?/2` accepts a `nil` body type, so a response could declare
`{200, %{optional(:"content-type") => :"application/pdf"}, nil}`, return bytes
anyway, and have them sent — while the generated spec documents no content for
that response at all. The JSON path already caught the mismatch through
`Spectral.encode`; the raw path passed it straight to `raw_response_body/2`.
Both paths now raise, which is what the declared type says and what the rest of
the response contract does on a mismatch. Found by Copilot's review of 60f11a9.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
`{:spectral, "~> 0.14.0"}` and `{:spectra, "~> 0.14.0"}` — spectral 0.14.0
requires spectra ~> 0.14.0, and the direct spectra dependency has to move with
it because both modules Record.extract from spectra_internal.hrl. The
constraints admit every later 0.14.x of either: resolution already picked up
spectra 0.14.1.
No source changes were needed — the records, Spectral.TypeInfo and
spectra_util:apply_args/3 this library uses are unchanged across the bump.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
There was a problem hiding this comment.
🔵 Needs a closer look
An unresolved nil-alias runtime inconsistency remains, with related documentation and test coverage updates needed.
Review details
Suppressed comments (5)
README.md:192
- This README bullet omits the intentional
nilexception: a non-JSON response with a declarednilbody type is accepted as an empty response by the implementation and tests. As written, “anything else” says that supported contract must raise; documentnilas the empty-body alternative.
- Under a non-JSON media type the body is sent as-is, so it must be typed `binary()`; declaring anything else raises when the spec is generated, and returning a non-binary raises on the request. `application/json` and `*+json` bodies are still encoded by Spectral, whatever the media type's casing.
lib/phoenix_spectral.ex:100
- This module documentation also says every non-JSON body type other than
binary()raises, butbinary_body_type?/2deliberately accepts anilbody type and the controller sends an empty body for it. Include that exception so the OpenAPI module docs do not contradict the supported response contract.
non-JSON media type is sent verbatim at runtime, so its body type must be `binary()`;
declaring anything else raises here rather than generating a spec the endpoint cannot
serve. See `PhoenixSpectral.Controller` for how such a body is sent.
lib/phoenix_spectral/controller.ex:95
- The moduledoc has the same contract mismatch: the implementation explicitly accepts
sp_literal(value: nil)for a non-JSON response, but “must be ...binary(); anything else raises” presentsnilas invalid. Please document the nil/empty-body exception here so the public controller documentation matches the tested behavior.
Under a non-JSON media type the body must be typed and returned as a `binary()`;
anything else raises. `application/json` and `*+json` bodies are still encoded by
`Spectral.encode`, whatever their casing, and only they get `; charset=utf-8` appended —
lib/phoenix_spectral/controller.ex:444
- A named body alias that resolves to
nilskips the fast path because this branch only matches the rawsp_literal(value: nil)record. A non-JSON response such as@type empty :: niltherefore reachesraw_response_body/2and raises when the action returnsnil, even thoughadd_response_body/5resolves the alias and accepts it. Resolve the body type for this runtime nil check too, or otherwise make alias-based nil responses consistent with generation.
else
{:ok, raw_response_body(body, content_type)}
lib/phoenix_spectral/internal.ex:94
- The new
binary_body_type?/2nil branch is only exercised through dispatch:download_emptyis not registered inTestContentTypeRouter, sogenerate_openapi/2never verifies that a non-JSON response with a declarednilbody is accepted and emits no content. Removing this branch would leave the generator suite green while breaking that valid contract; add the route and an OpenAPI assertion for the empty response.
def binary_body_type?(sp_literal(value: nil), _type_info), do: true
- Files reviewed: 14/16 changed files
- Comments generated: 0 new
- Review effort level: Lite
A nil body type means "no body": the OpenAPI response carries no content and the runtime sends an empty body. An alias to nil got neither — the generator emitted a "null" enum under the declared media type, and the runtime encoded "null" or, under a non-JSON media type, raised because nil is not a binary. binary_body_type?/2 accepted such an alias, so the generator waved a declaration through that the runtime then rejected. Both paths now resolve the body type before deciding, through the resolve_body_type/2 the two body checks already needed, and the docs name nil as the exception to "a non-JSON body must be a binary()". The generator branch for a nil body type was reachable only from the runtime suite; both empty actions are routed now and asserted to emit no content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
Claude Code discovers skills by SKILL.md; the lowercase filename kept /release from being listed at all, unlike the two skills beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD
Problem
Every response body was emitted under
application/json:add_responses/3always calledSpectral.OpenAPI.response_with_body/3, even thoughresponse_with_body/4takes a content type. An action streaming PDF or XML bytes could not describe itself — the spec claimed a JSON string, so a generated client sendsAccept: application/jsonand expects JSON. Reported from Repejo, where three download endpoints serveapplication/pdf/application/xml.Solution
A
content-typeentry in a response headers map, with the media type as a literal atom, is now that response's media type declaration:The 200 is now emitted as
This is the headers-map variant of option 2 in the report. Options 1 and 3 were rejected:
spectral/1function metadata accepts onlysummary,descriptionanddeprecated(spectra raises{invalid_spectra_field, …}on anything else), and silently re-readingbinary()asapplication/octet-streamwould change what existing endpoints advertise and send.connfor streaming or file sends. The response carriescontent-type: application/pdf, set from that declaration; the OpenAPI response keys the body under the media type rather than listing it in itsheadersobject. Every other entry in the map is still emitted as a response header.requiredand omitted from the returned map raises, and so does a returned value that does not match the declared media type.optional(:"content-type")lets an action leave it out and take the media type from the typespec alone — which is what aconn-returning action does.binary(). Declaring another body type raises while the spec is generated rather than documenting a response the endpoint cannot serve; returning a non-binary raises on the request.application/jsonand*+jsonbodies are still encoded by Spectral. Only the type and subtype decide that, so casing and parameters (:"application/json; charset=utf-8") behave.; charset=utf-8appended; bake a charset into the atom (:"text/csv; charset=utf-8") when a text format needs one.Breaking: a
content-typeentry whose value is not a literal atom media type —%{"content-type": String.t()}, which 0.6.x documented as a response header and then overwrote withapplication/json— now raises in both paths. Two narrower behaviour changes come with the validation above: arequireddeclaration the action omits, and a returned value that disagrees with the declaration, now raise where 0.6.x ignored them. The changelog says so under Changed; this wants a minor bump, not a patch.Type reference resolution, and two bugs it hid
Reading a headers map and checking a body type both have to resolve type references, and each carried its own copy of the same two clauses. They now share
PhoenixSpectral.Internal.resolve_type_ref/2, which returns the resolved type together with the type info it belongs to — the pair has to travel, since a remote reference resolves in its own module's type info.spectra has a
resolve_type_ref/2of its own (spectra.erl), but it is not exported and covers only local references; remote ones it resolves per format module, with codec andonly/field_aliasesmeta handling this library deliberately does not replicate. The piece worth borrowing isspectra_util:apply_args/3, which substitutes a reference's arguments for the alias's variables.That fixes a bug the duplication hid: both sites looked a user type reference up at arity 0, so a parameterized alias —
@type tagged(t) :: %{required(:"x-total-count") => t}as a headers map, orpayload(binary())as a body — died on the{:ok, _} =match. The reference carries its own arity, and that is now what is used.nilbody types, directly or through an aliasA
nilbody type means the response has no body: spectra emits nocontentfor it, so the response is sent empty, and returning a body anyway now raises rather than sending bytes the spec does not describe.An alias to
nil(@type empty :: nil) got neither half of that. The generator emittedcontent: {"application/pdf": {"schema": {"enum": ["null"]}}}, and the runtime encoded"null"or — under a declared non-JSON media type — raised, becausenilis not a binary.binary_body_type?/2resolves aliases, so the generator waved through exactly the declaration the runtime then rejected. Both paths resolve the body type before deciding now, through theresolve_body_type/2the two body checks already shared, and an alias tonilbehaves like a barenileverywhere. Note the JSON side of that: a{204, %{}, empty()}response now carries nocontentand sends an empty body, where before it documented and sent"null".The generator's own
nilbranch turned out to be reachable only from the runtime suite — neither empty action was routed — so both are routed now and asserted to emit no content. Changelog entries under Fixed.Changes
PhoenixSpectral.Internalgainsresolve_type_ref/2,resolve_body_type/2,pop_content_type/2,response_content_type/2,content_type_header?/1,json_content_type?/1andbinary_body_type?/2.PhoenixSpectralroutes the declared type intoSpectral.OpenAPI.response_with_body/4, rejects a body type the media type cannot carry, and collapses a body type that resolves tonil.PhoenixSpectral.Controllervalidates the declaredcontent-typealongside the other response headers, sets the response's content type from the declaration, sends raw bodies verbatim, and sends an empty body for anilbody type while rejecting a body returned under one.nilas the exception to "a non-JSON body must be abinary()".Tests
124 in the root suite, 17 in the example app, which gains a
text/vcarddownload endpoint exercising a parameterized media type over real HTTP. Each guard was checked by mutation: deleting the casing fold, the*+jsonbranch, the parameter strip, the body-type check, the multiple-entry clause, the media-type shape check or either half of thenilresolution fails at least one test. The two validation tests fail on the pre-56e8715 runtime, the two parameterized-alias tests on the pre-60f11a9 resolution, thenilbody-type test on the pre-1b9c0bd raw path, and the four alias-to-niltests on the pre-9dc372f generator and runtime.Verified on OTP 28.2 / Elixir 1.20.0 and OTP 27 / Elixir 1.18.4:
mix compile --force --warnings-as-errors,mix test,mix credo --strict,mix ex_dna,mix dialyzer,mix format --check-formatted, and the example suite.Repejo's own download endpoints are untouched — they can adopt this once a release with it is out.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QovEtXhV2U3w3qnCBoyhiD