Skip to content

docs(core): correct why authFetch tolerates a non-JSON body - #152

Merged
Bccorb merged 1 commit into
mainfrom
docs/json-tolerance-rationale
Aug 31, 2026
Merged

docs(core): correct why authFetch tolerates a non-JSON body#152
Bccorb merged 1 commit into
mainfrom
docs/json-tolerance-rationale

Conversation

@Bccorb

@Bccorb Bccorb commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Follow-up to fells-code/seamless-auth-api#244 (closing fells-code/seamless-auth-api#241), which made a 429 answer the JSON error shape.

Answering the question that prompted this: no, the shim cannot go

The ask was to remove makeJsonTolerant once the 429 was fixed. It is not safe, and this PR makes the code say why instead.

The comment currently reads:

Upstream responses aren't always JSON: a rate-limited request comes back as plain text ("Too many requests…") and a 204 has no body.

The first half is no longer true. Leaving it there is worse than saying nothing, because the next reader checks the stated reason, finds it fixed, and concludes the shim is dead code.

Two reasons survive:

  1. A 204 has no body. GET /magic-link/check answers 204 while a link is unconfirmed. pollMagicLinkConfirmationHandler guards that status explicitly, but the tolerance is what makes the guard optional rather than mandatory at 24 call sites.
  2. The API sits behind a load balancer. A 502 or a gateway timeout is that proxy's HTML error page, produced without the API being involved at all. No change upstream can prevent it, because the API never sees the request.

I checked all 24 .json() call sites in core. Every one parses a body before checking the status, so any of them would throw on either case. After the 429 fix, every response the API itself produces is JSON, and the API's only 204s are both in pollMagicLinkConfirmation, so reason 1 is narrow. Reason 2 is not, and is outside anyone's control here.

What changed

  • The comment, replaced with the two reasons that hold plus a note that the rate-limit case is fixed and removed a reason rather than the shim.
  • The test that pinned the non-JSON path was named "e.g. a 429" and asserted on "Too many requests…". After #244 that is the one status that can no longer produce a non-JSON body, so the test was anchored on an example that had stopped being an example. Re-anchored on a 502 HTML error page.

No behaviour change. makeJsonTolerant is untouched and no changeset is needed.

Verification

pnpm build clean, pnpm test at the workspace root: core 231, express 154, fastify 48, all passing.

The comment named a rate-limited request as a live reason. That is fixed
upstream: a 429 has answered the JSON error shape since seamless-auth-api#241,
so anyone reading this now would find the stated reason no longer true and could
reasonably conclude the shim can go.

It cannot. A 204 still carries no body, which is what GET /magic-link/check
answers while a link is unconfirmed, and the auth API sits behind a load
balancer, so a 502 or a gateway timeout arrives as that proxy's HTML error page
without the API being involved at all. No upstream change can prevent that one.
Twenty-four call sites parse a body before checking the status, and every one of
them would throw.

The test that pinned the non-JSON path used a 429 as its example, which is now
the one status that cannot produce it. Re-anchored on a proxy error page, which
is the case that actually survives.

No behaviour change.
@Bccorb
Bccorb merged commit 8e90cf8 into main Aug 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant