From ee988ad6234bd7310500a7a1c7616aa89a9dca6a Mon Sep 17 00:00:00 2001 From: Pedro Bini Date: Tue, 18 Aug 2026 13:36:03 -0300 Subject: [PATCH] [NO-REF] Preserve HTTP error details when the response body is not JSON `throwHttpErrorFromResponse` parsed every non-2XX body as JSON. Error responses are not always JSON: rate limit and gateway responses are commonly plain text or HTML. For those, `response.json()` rejected and the SDK surfaced the parse failure instead of the request failure: SyntaxError: Unexpected token 'T', "Too many requests" is not valid JSON at Response.json (node_modules/node-fetch/src/body.js:149:15) The real status, statusText, url and headers were lost with it, which are the details the function exists to attach. Read the body as text and attempt to parse it, falling back to the raw text as the error message, and to `HTTP ` when the body is empty. `error.message` is now always a string, where previously it was `undefined` for a JSON body without a `message` field. All modules route their non-2XX responses through this helper, so this covers every endpoint. `tracker.js` already handled non-JSON bodies this way when emitting error events. Co-Authored-By: Claude Opus 5 --- spec/src/utils/helpers.js | 61 +++++++++++++++++++++++++++++---------- src/utils/helpers.js | 25 ++++++++++++++-- 2 files changed, 67 insertions(+), 19 deletions(-) diff --git a/spec/src/utils/helpers.js b/spec/src/utils/helpers.js index 2d7d1a7c..e8db295e 100644 --- a/spec/src/utils/helpers.js +++ b/spec/src/utils/helpers.js @@ -145,22 +145,51 @@ describe('ConstructorIO - Utils - Helpers', () => { }, }; - try { - await throwHttpErrorFromResponse(new Error(), { - json: () => new Promise((resolve) => { - resolve({ - message: errorMessage, - }); - }), - ...responseData, - }); - } catch (e) { - expect(e.message).to.equal(errorMessage); - expect(e.status).to.equal(responseData.status); - expect(e.statusText).to.equal(responseData.statusText); - expect(e.url).to.equal(responseData.url); - expect(e.headers).to.deep.equal(responseData.headers); - } + const error = await throwHttpErrorFromResponse(new Error(), { + text: () => Promise.resolve(JSON.stringify({ message: errorMessage })), + ...responseData, + }).catch((e) => e); + + expect(error.message).to.equal(errorMessage); + expect(error.status).to.equal(responseData.status); + expect(error.statusText).to.equal(responseData.statusText); + expect(error.url).to.equal(responseData.url); + expect(error.headers).to.deep.equal(responseData.headers); + }); + + it('Should throw an error with the raw body when the response is not JSON', async () => { + const responseData = { + status: 429, + statusText: 'Too Many Requests', + url: 'https://constructor.io', + headers: { + 'retry-after': '30', + }, + }; + + const error = await throwHttpErrorFromResponse(new Error(), { + text: () => Promise.resolve('Too many requests'), + ...responseData, + }).catch((e) => e); + + expect(error.message).to.equal('Too many requests'); + expect(error.status).to.equal(responseData.status); + expect(error.statusText).to.equal(responseData.statusText); + expect(error.url).to.equal(responseData.url); + expect(error.headers).to.deep.equal(responseData.headers); + }); + + it('Should throw an error with a status fallback when the response body is empty', async () => { + const error = await throwHttpErrorFromResponse(new Error(), { + text: () => Promise.resolve(''), + status: 502, + statusText: 'Bad Gateway', + url: 'https://constructor.io', + headers: {}, + }).catch((e) => e); + + expect(error.message).to.equal('HTTP 502'); + expect(error.status).to.equal(502); }); }); diff --git a/src/utils/helpers.js b/src/utils/helpers.js index 4428e06e..80f68976 100644 --- a/src/utils/helpers.js +++ b/src/utils/helpers.js @@ -48,15 +48,34 @@ const utils = { return snakeCasedObj; }, - throwHttpErrorFromResponse: (error, response) => response.json().then((json) => { - error.message = json.message; + // Attach the details of a non-2XX response to an error and throw it + // - Error bodies are not always JSON: rate limit and gateway responses are + // commonly plain text or HTML, so attempting to parse and falling back to + // the raw body keeps the real status and message instead of surfacing a + // SyntaxError from the parse itself + throwHttpErrorFromResponse: async (error, response) => { + let message = ''; + + try { + message = await response.text(); + + const parsed = JSON.parse(message); + + if (parsed && typeof parsed.message === 'string') { + message = parsed.message; + } + } catch (e) { + // Body is either unreadable or not JSON - keep whatever text we have + } + + error.message = message.trim() || `HTTP ${response.status}`; error.status = response.status; error.statusText = response.statusText; error.url = response.url; error.headers = response.headers; throw error; - }), + }, isNil: (value) => value == null,