fix: read error response bodies once in the TypeScript client#39
Merged
Conversation
A fetch Response body is a one-shot stream. The error paths in the generated client and OAuth2 helper called response.json() and fell back to response.text() on the same response, so any non-JSON error body (e.g. a plain-text 404 from the gateway) crashed with "Body is unusable: Body has already been read" instead of surfacing the real status and body. Read the body once as text and attempt JSON.parse, and add generated regression tests that use real Response objects so one-shot body semantics are actually exercised.
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.
Problem
A fetch
Responsebody is a one-shot stream. The error paths in the generated TypeScript client calledresponse.json()and, when that failed, fell back toresponse.text()on the same response. Any non-JSON error body (for example a plain-text 404 from the gateway) therefore crashed with:In
Client.requestthat TypeError escaped to the outer catch and was rethrown asApiErrorwith status0/NETWORK_ERROR, completely masking the real HTTP status and body. In practice this means every non-JSON API error a consumer hits is unreadable.The same pattern (
response.json().catch(() => response.text())) existed twice in the OAuth2 token flows.The Python client is not affected:
requestsbuffers the body, so.textremains readable after a failed.json().Fix
xdk-gen/templates/typescript/main_client.j2: read the error body once as text, attemptJSON.parse, and fall back to the raw text. TheApiErrornow always carries the real status, status text, and body. For JSON error bodies the behavior is unchanged.xdk-gen/templates/typescript/oauth2_auth.j2: read the error body once as text in both token-exchange error paths.Tests
New generated test file
tests/client_errors.test.ts(templatetest_client_errors.j2, registered ingenerator.rs) coveringClient.requestand both OAuth2 flows with JSON, plain-text, and empty error bodies. The tests use realResponseobjects rather than mocks with independentjson()/text()resolvers — the existing generic tests mock those methods separately, which is exactly why this bug was never caught. Verified the non-JSON-body test fails against the previous implementation and passes with the fix.Validation:
cargo testgreen, SDK regenerated withcargo run -- typescript --latest true,npm run buildgreen, full jest suite 1153/1153 passing.