From 98db74bdbc5be272e425f0d89f1385229ad2ced3 Mon Sep 17 00:00:00 2001 From: Santiago Medina Rolong Date: Fri, 24 Jul 2026 21:33:45 -0700 Subject: [PATCH] fix: read error response bodies once in the TypeScript client 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. --- xdk-gen/src/typescript/generator.rs | 1 + xdk-gen/templates/typescript/main_client.j2 | 16 +- xdk-gen/templates/typescript/oauth2_auth.j2 | 14 +- .../typescript/test_client_errors.j2 | 177 ++++++++++++++++++ 4 files changed, 201 insertions(+), 7 deletions(-) create mode 100644 xdk-gen/templates/typescript/test_client_errors.j2 diff --git a/xdk-gen/src/typescript/generator.rs b/xdk-gen/src/typescript/generator.rs index ef6d9d88..5fbd10b0 100644 --- a/xdk-gen/src/typescript/generator.rs +++ b/xdk-gen/src/typescript/generator.rs @@ -85,6 +85,7 @@ language! { render "test_contracts" => "tests/{}/test_contracts.test.ts", render "test_pagination" => "tests/{}/test_pagination.test.ts" }, + render "test_client_errors" => "tests/client_errors.test.ts", render "jest.config" => "jest.config.cjs" ] } diff --git a/xdk-gen/templates/typescript/main_client.j2 b/xdk-gen/templates/typescript/main_client.j2 index d45be528..73241574 100644 --- a/xdk-gen/templates/typescript/main_client.j2 +++ b/xdk-gen/templates/typescript/main_client.j2 @@ -396,13 +396,23 @@ export class Client { }); if (!response.ok) { + // Read the body exactly once: a fetch Response body is a one-shot + // stream, so a failed json() followed by text() would throw + // "Body is unusable" and mask the real error. let errorData: any; try { - errorData = await response.json(); + const errorText = await response.text(); + try { + errorData = JSON.parse(errorText); + } catch { + errorData = errorText; + } } catch { - errorData = await response.text(); + // The body could not be read at all (e.g. an aborted stream); + // still surface the status line below. + errorData = undefined; } - + throw new ApiError( errorData && errorData.message ? errorData.message : `HTTP ${response.status}: ${response.statusText}`, response.status, diff --git a/xdk-gen/templates/typescript/oauth2_auth.j2 b/xdk-gen/templates/typescript/oauth2_auth.j2 index 81e5b116..5cf0995e 100644 --- a/xdk-gen/templates/typescript/oauth2_auth.j2 +++ b/xdk-gen/templates/typescript/oauth2_auth.j2 @@ -113,8 +113,11 @@ export class OAuth2 { }); if (!response.ok) { - const errorData = await response.json().catch(() => response.text()); - throw new Error(`HTTP error! status: ${response.status}, body: ${JSON.stringify(errorData)}`); + // Read the body exactly once: a fetch Response body is a one-shot + // stream, so json() followed by text() on the same response throws + // "Body is unusable" and masks the real error. + const errorText = await response.text().catch(() => ''); + throw new Error(`HTTP error! status: ${response.status}, body: ${errorText}`); } const data = await response.json(); @@ -167,8 +170,11 @@ export class OAuth2 { }); if (!response.ok) { - const errorData = await response.json().catch(() => response.text()); - throw new Error(`Failed to refresh token: ${response.status}, body: ${JSON.stringify(errorData)}`); + // Read the body exactly once: a fetch Response body is a one-shot + // stream, so json() followed by text() on the same response throws + // "Body is unusable" and masks the real error. + const errorText = await response.text().catch(() => ''); + throw new Error(`Failed to refresh token: ${response.status}, body: ${errorText}`); } const data = await response.json(); diff --git a/xdk-gen/templates/typescript/test_client_errors.j2 b/xdk-gen/templates/typescript/test_client_errors.j2 new file mode 100644 index 00000000..99f06388 --- /dev/null +++ b/xdk-gen/templates/typescript/test_client_errors.j2 @@ -0,0 +1,177 @@ +/** + * AUTO-GENERATED FILE - DO NOT EDIT + * This file was automatically generated by the XDK build tool. + * Any manual changes will be overwritten on the next generation. + * + * Auto-generated error-handling tests for Client.request() and the OAuth2 + * token flows. This module contains tests that validate how non-OK HTTP + * responses are surfaced: JSON bodies, plain-text bodies, and empty bodies. + * + * The tests use real Response objects rather than mocks with independent + * json()/text() resolvers, because a fetch Response body is a one-shot + * stream: reading it twice throws "Body is unusable". A mock with separate + * resolvers would hide exactly the class of bug these tests guard against. + * + * Generated automatically - do not edit manually. + */ + +import { describe, it, expect, afterEach, jest } from '@jest/globals'; +import { Client, ApiError } from '../src/client.js'; +import { OAuth2 } from '../src/oauth2_auth.js'; + +/** Calls an async function that must reject, and returns the rejection. */ +const captureRejection = async (fn: () => Promise): Promise => { + try { + await fn(); + } catch (error) { + return error; + } + throw new Error('expected the call to reject'); +}; + +describe('Client.request error handling', () => { + const client = new Client({ + baseUrl: 'https://api.example.com', + bearerToken: 'test-token', + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + const mockResponse = (response: Response) => { + jest + .spyOn(client.httpClient, 'request') + .mockResolvedValue(response as any); + }; + + it('surfaces a non-JSON error body as the ApiError data', async () => { + mockResponse( + new Response('Route not found', { status: 404, statusText: 'Not Found' }) + ); + + const error = await captureRejection(() => + client.request('GET', '/2/example') + ); + + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(404); + expect(error.statusText).toBe('Not Found'); + expect(error.message).toBe('HTTP 404: Not Found'); + expect(error.data).toBe('Route not found'); + }); + + it('parses a JSON error body into the ApiError data', async () => { + const body = { + title: 'Too Many Requests', + detail: 'Rate limit exceeded', + status: 429, + }; + mockResponse( + new Response(JSON.stringify(body), { + status: 429, + statusText: 'Too Many Requests', + }) + ); + + const error = await captureRejection(() => + client.request('GET', '/2/example') + ); + + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(429); + expect(error.data).toEqual(body); + }); + + it('uses the message field of a JSON error body as the error message', async () => { + mockResponse( + new Response(JSON.stringify({ message: 'Invalid cursor' }), { + status: 400, + statusText: 'Bad Request', + }) + ); + + const error = await captureRejection(() => + client.request('GET', '/2/example') + ); + + expect(error).toBeInstanceOf(ApiError); + expect(error.message).toBe('Invalid cursor'); + expect(error.status).toBe(400); + }); + + it('still reports the status when the error body is empty', async () => { + mockResponse( + new Response(null, { status: 500, statusText: 'Internal Server Error' }) + ); + + const error = await captureRejection(() => + client.request('GET', '/2/example') + ); + + expect(error).toBeInstanceOf(ApiError); + expect(error.status).toBe(500); + expect(error.message).toBe('HTTP 500: Internal Server Error'); + }); +}); + +describe('OAuth2 token flow error handling', () => { + const oauth2 = new OAuth2({ + clientId: 'test-client-id', + redirectUri: 'https://example.com/callback', + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + const mockFetch = (response: Response) => { + jest + .spyOn(globalThis, 'fetch') + .mockResolvedValue(response as any); + }; + + it('surfaces a non-JSON error body from the code exchange', async () => { + mockFetch( + new Response('Service Unavailable', { + status: 503, + statusText: 'Service Unavailable', + }) + ); + + const error = await captureRejection(() => + oauth2.exchangeCode('test-code') + ); + + expect(error.message).toBe( + 'HTTP error! status: 503, body: Service Unavailable' + ); + }); + + it('surfaces a non-JSON error body from the token refresh', async () => { + mockFetch( + new Response('Bad Gateway', { status: 502, statusText: 'Bad Gateway' }) + ); + + const error = await captureRejection(() => + oauth2.refreshToken('test-refresh-token') + ); + + expect(error.message).toBe( + 'Failed to refresh token: 502, body: Bad Gateway' + ); + }); + + it('surfaces a JSON error body from the code exchange', async () => { + const body = JSON.stringify({ error: 'invalid_grant' }); + mockFetch( + new Response(body, { status: 400, statusText: 'Bad Request' }) + ); + + const error = await captureRejection(() => + oauth2.exchangeCode('test-code') + ); + + expect(error.message).toBe(`HTTP error! status: 400, body: ${body}`); + }); +});