fix: don't crash on non-JSON upstream responses (#41)#43
Merged
Conversation
authFetch now parses response bodies defensively: a plain-text error (e.g. a
rate-limited 429) or an empty body (204) no longer throws in handlers that read
the body before checking status, which previously crashed the adapter process.
Non-JSON -> { message: text }, empty -> undefined. Adds authFetch regression tests.
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.
Fixes #41.
Problem
Every
@seamless-auth/corehandler (and the express proxy) callsawait response.json()before checking the status. Upstream responses aren't always JSON — a rate-limited request
returns plain text (
Too many requests, please try again later.) and a204has no body —so
json()throws, the rejection goes unhandled, and the adapter process crashes. A userhitting a rate limit mid-flow can take down the adapter.
Fix
Centralize it in
authFetch(the single fetch chokepoint used by core handlers and theexpress proxy): the returned response's
json()is made tolerant — it reads the body as textand returns parsed JSON,
{ message: <text> }for a non-JSON body, orundefinedfor anempty one. No handler changes needed; the crash is removed at the source.
A guard keeps existing tests (which mock
fetchwith a bare object) working.Tests
authFetchregression cases: non-JSON 429 →{ message }(no throw), valid JSON parsesnormally, empty 204 →
undefined.seamless verify --localis 22/22 green against the fixed SDKs.