From 65f14ca3bba5bfa02ee95ef5386405b7d88a0ed2 Mon Sep 17 00:00:00 2001 From: Pedro Bini Date: Tue, 18 Aug 2026 13:22:43 -0300 Subject: [PATCH 1/5] [NO-REF] Vendor catalog test fixtures instead of fetching them The catalog and tasks specs downloaded their fixture files from raw.githubusercontent.com in a `before` hook on every run. When the request is rate limited, the response body is plain text rather than a file, which surfaces as `SyntaxError: Unexpected token 'T', "Too many requests" is not valid JSON` and a cascade of 5s timeouts unrelated to the code under test. Commit the four fixtures (1.9 KB total) under `spec/fixtures/` and read them from disk via a `readCatalogFixture` helper, removing the network dependency from test setup. Co-Authored-By: Claude Opus 5 --- spec/fixtures/catalog.tar.gz | Bin 0 -> 622 bytes spec/fixtures/item_groups.csv | 10 +++++++++ spec/fixtures/items.csv | 12 +++++++++++ spec/fixtures/variations.csv | 3 +++ spec/mocha.helpers.js | 6 ++++++ spec/src/modules/catalog/catalog-files.js | 24 ++++------------------ spec/src/modules/tasks.js | 6 +----- 7 files changed, 36 insertions(+), 25 deletions(-) create mode 100644 spec/fixtures/catalog.tar.gz create mode 100644 spec/fixtures/item_groups.csv create mode 100644 spec/fixtures/items.csv create mode 100644 spec/fixtures/variations.csv diff --git a/spec/fixtures/catalog.tar.gz b/spec/fixtures/catalog.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..4d36d0d461fdaf6776db2aab57db8cd4b726955a GIT binary patch literal 622 zcmV-!0+Ib6iwFP;jsIi-1MQa0j@mE~fcKoI$O9O#osa;xWXnq3wwJc5U@yoe-jd?{ z$abh!+Cv|tPq$A}$8jR)Vz#OZQmc%FCb7p~#vacMsic`0uEp-^d;%d1eBZ5g7zA2F z+`QL<0vx!=_roxR0mRsa$b%lda^V?$>ZB5qaODcksW@e)%+JN=JHnt)VY zfmKd23M$UP7hY85Doq5qCu=Ha*99xMd5$^B6Od6!5+cb=QXy|r$`S|FVFa%g2!V~1 zrI?LI_3Ut6WTSG_On;CMj7A`46$Ojs9oVEvLURrHMAV?BU`@d`3I^JT#S%PF#)@y? zIK|Tx&qnbYUt&K=?DZY*3H@-xzD^|`}bH+B#xZ4w_iTz{(O$lt2z)(~G zDb4MeP-8+HCTx$U!C3b+(U^%1GcnPAG)MILWYDJ?HnqW~=AdIBQnsTUyqe!U2eB#@ zlGA)k`QR&AZ>b!-!TxxR5S~s4Yfec9ZuPkP^!>;Cbhjl+yKmJp_hYlpZLC^15}FmY z8yV&5_7?>c+9{fke&buijm#KO;*^x+l^h_@JRI9)XA-e`jdQvD-w~b?nHKpOo4^b8 zA9@IN^&cw(hz;qU0`Ow~L-fo4KOcLj&;LtENmL$5o%9K)Q{K@%Nd25z z>&7dL(j&UB3!|L<`N*h)!G%%I!P~Oc&cVvK)z1D`BBKrlW25@K@1>VsdigW?3C?uk Ie*h8y0OT|_&Hw-a literal 0 HcmV?d00001 diff --git a/spec/fixtures/item_groups.csv b/spec/fixtures/item_groups.csv new file mode 100644 index 00000000..43180b3b --- /dev/null +++ b/spec/fixtures/item_groups.csv @@ -0,0 +1,10 @@ +parent_id,id,name +,All,All +All,Brands,Brands +All,Styles,Styles +Brands,BrandA,BrandA +Brands,BrandX,BrandX +BrandX,BrandXY,BrandXY +BrandA,BrandAB,BrandAB +Styles,StyleA,StyleA +Styles,StyleB,StyleB diff --git a/spec/fixtures/items.csv b/spec/fixtures/items.csv new file mode 100644 index 00000000..be964b7b --- /dev/null +++ b/spec/fixtures/items.csv @@ -0,0 +1,12 @@ +id,item_name,url,group_ids,facet:Color,facet:Brand,metadata:testField +10001,Item1,https://test.com/p/10001,StyleA,Blue,XYZ,hiddenFieldValue +10002,Item2,https://test.com/p/10002,BrandXY,yellow,,hiddenFieldValue +10003,Item3,https://test.com/p/10003,StyleB,pink,ABC,hiddenFieldValue +10004,Item4,https://test.com/p/10004,BrandA,yellow,ABC,hiddenFieldValue +10005,Item5,https://test.com/p/10005,BrandXY,red,XYZ,hiddenFieldValue +10006,Item6,https://test.com/p/10006,BrandX,green,XYZ,hiddenFieldValue +10007,Item7,https://test.com/p/10007,StyleB,green,ABC,hiddenFieldValue +10008,Item8,https://test.com/p/10008,StyleA|BrandA,blue,XYZ,hiddenFieldValue +10009,Item9,https://test.com/p/10009,BrandAB,silver,DEF,hiddenFieldValue +luistrenker-jacket-K245511299-cream,Jacket “Rivka“ Luis Trenker cream,https://demo.commercetools.com/en/luistrenker-jacket-K245511299-cream.html,,,, +peanut-1,peanut,https://test.com/p/peanut-1,All,,, diff --git a/spec/fixtures/variations.csv b/spec/fixtures/variations.csv new file mode 100644 index 00000000..ccec519a --- /dev/null +++ b/spec/fixtures/variations.csv @@ -0,0 +1,3 @@ +variation_id,item_id,item_name,url,facet:Color +20001,10001,item1 variation,https://test.com/p/20001,red|Blue +M0E20000000E2ZJ,luistrenker-jacket-K245511299-cream,Jacket “Rivka“ Luis Trenker cream,, diff --git a/spec/mocha.helpers.js b/spec/mocha.helpers.js index 0c4c2c87..12319b93 100644 --- a/spec/mocha.helpers.js +++ b/spec/mocha.helpers.js @@ -1,6 +1,11 @@ /* eslint-disable import/no-unresolved */ +const fs = require('fs'); +const path = require('path'); const qs = require('qs'); +// Read a vendored catalog fixture as a buffer +const readCatalogFixture = (name) => fs.readFileSync(path.join(__dirname, 'fixtures', name)); + // Extract query parameters as object from url const extractUrlParamsFromFetch = (fetch) => { const lastCallArguments = fetch && fetch.args && fetch.args[fetch.args.length - 1]; @@ -53,6 +58,7 @@ const extractUrlFromFetch = (fetch) => { }; module.exports = { + readCatalogFixture, extractUrlParamsFromFetch, extractBodyParamsFromFetch, extractHeadersFromFetch, diff --git a/spec/src/modules/catalog/catalog-files.js b/spec/src/modules/catalog/catalog-files.js index f9b881e4..d031ebde 100644 --- a/spec/src/modules/catalog/catalog-files.js +++ b/spec/src/modules/catalog/catalog-files.js @@ -54,31 +54,15 @@ describe('ConstructorIO - Catalog', () => { // Ensure Mocha doesn't time out waiting for operation to complete this.timeout(10000); - const catalogExamplesBaseUrl = 'https://raw.githubusercontent.com/Constructor-io/integration-examples/main/catalog/'; - let itemsBuffer = null; + const itemsBuffer = helpers.readCatalogFixture('items.csv'); + const variationsBuffer = helpers.readCatalogFixture('variations.csv'); + const itemGroupsBuffer = helpers.readCatalogFixture('item_groups.csv'); + const tarArchiveBuffer = helpers.readCatalogFixture('catalog.tar.gz'); let itemsStream = null; - let variationsBuffer = null; let variationsStream = null; - let itemGroupsBuffer = null; let itemGroupsStream = null; - let tarArchiveBuffer = null; let tarArchiveStream = null; - before(async () => { - // Grab catalog files from Integration Examples repo - const itemsResponse = await nodeFetch(`${catalogExamplesBaseUrl}items.csv`); - itemsBuffer = await itemsResponse.buffer(); - - const variationsResponse = await nodeFetch(`${catalogExamplesBaseUrl}variations.csv`); - variationsBuffer = await variationsResponse.buffer(); - - const itemGroupsResponse = await nodeFetch(`${catalogExamplesBaseUrl}item_groups.csv`); - itemGroupsBuffer = await itemGroupsResponse.buffer(); - - const tarArchiveResponse = await nodeFetch(`${catalogExamplesBaseUrl}catalog.tar.gz`); - tarArchiveBuffer = await tarArchiveResponse.buffer(); - }); - beforeEach(async () => { itemsStream = createStreamFromBuffer(itemsBuffer); variationsStream = createStreamFromBuffer(variationsBuffer); diff --git a/spec/src/modules/tasks.js b/spec/src/modules/tasks.js index 3fa285dc..7f7460eb 100644 --- a/spec/src/modules/tasks.js +++ b/spec/src/modules/tasks.js @@ -36,12 +36,8 @@ describe('ConstructorIO - Tasks', function ConstructorIOTasks() { ...validOptions, }); - // Grab items file from Integration Examples repo and upload - const itemsResponse = await nodeFetch('https://raw.githubusercontent.com/Constructor-io/integration-examples/main/catalog/items.csv'); - const itemsBuffer = await itemsResponse.buffer(); - const data = { - items: itemsBuffer, + items: helpers.readCatalogFixture('items.csv'), section: 'Products', }; From 3528d3ece96addd9790a9551377d6b55ea748064 Mon Sep 17 00:00:00 2001 From: Pedro Bini Date: Tue, 18 Aug 2026 13:36:03 -0300 Subject: [PATCH 2/5] [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, From 995eacbe9043f83112e0ed5b1fe35b71c7bbcdc8 Mon Sep 17 00:00:00 2001 From: Pedro Bini Date: Tue, 18 Aug 2026 13:44:41 -0300 Subject: [PATCH 3/5] [NO-REF] Raise the mocha timeout to 15s The suite makes real API calls, and the global 5s timeout is not enough for the slower endpoints when the run is being rate limited. Requests that complete normally take 50-350ms, so anything approaching 5s is already backing off rather than working. Across four runs on this branch the timeouts landed in seven different spec files, so this raises the shared timeout rather than patching each `describe` that happened to lose the race. Hook timeouts matter most here: `--retries 3` does not apply to hooks, so one slow `before` takes out its entire block on the first attempt. Co-Authored-By: Claude Opus 5 --- .mocharc.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mocharc.json b/.mocharc.json index 61b221d5..1b811433 100644 --- a/.mocharc.json +++ b/.mocharc.json @@ -1,5 +1,5 @@ { "recursive": true, - "timeout": 5000, + "timeout": 15000, "require": ["./spec/mocha.setup.js"] } From f3cb7ae710e13b1ba91e29bd9c4808588373a6cf Mon Sep 17 00:00:00 2001 From: Pedro Bini Date: Tue, 18 Aug 2026 13:49:52 -0300 Subject: [PATCH 4/5] [NO-REF] Run the test suite serially in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the 15s timeout from the previous commit: it made things worse (987 passing / 7 failing, against 1049 / 6 at 5s) because a rate limited request holding a slot for 15s prolongs the pressure instead of easing it. Every spec throttles itself with a 300ms `afterEach` delay, commented as being there to avoid rate limiting. One request per 300ms is ~200/min, which sits just under the `x-ratelimit-limit: 201` the API reports — the throttle is calibrated to the limit. `--parallel` gives each worker its own 300ms throttle, so N workers make N times the calibrated rate and the run exceeds the quota. That is why the failures move between spec files run to run, and why they surface as `Too many requests` and 5s-plus response times rather than as assertion failures. Running serially restores the rate the throttle was written for. This trades wall clock time for a run that can actually pass. Co-Authored-By: Claude Opus 5 --- .github/workflows/run-tests.yml | 2 +- .mocharc.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index b5975b3d..da6beb91 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -22,7 +22,7 @@ jobs: - name: Install dependencies run: npm install - name: Run tests - run: npm run test:parallel + run: npm test env: TEST_REQUEST_API_KEY: ${{ secrets.TEST_REQUEST_API_KEY }} TEST_CATALOG_API_KEY: ${{ secrets.TEST_CATALOG_API_KEY }} diff --git a/.mocharc.json b/.mocharc.json index 1b811433..61b221d5 100644 --- a/.mocharc.json +++ b/.mocharc.json @@ -1,5 +1,5 @@ { "recursive": true, - "timeout": 15000, + "timeout": 5000, "require": ["./spec/mocha.setup.js"] } From 8840b12545f6b44a9384deb9ec626d35dcf2ad1c Mon Sep 17 00:00:00 2001 From: Pedro Bini Date: Tue, 18 Aug 2026 14:07:46 -0300 Subject: [PATCH 5/5] [NO-REF] Surface the real failure in the preFilterExpression search test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test's promise chain had no rejection handler, so a failing assertion never reached `done()` and the failure was reported as a timeout instead. That is why it exceeded both the 5s and the 15s timeout identically: the duration was never the problem, `done()` simply was never called. Adding `.catch(done)` reports the actual assertion error. This is diagnostic — the underlying expectation still needs fixing once CI shows what the endpoint returns. Co-Authored-By: Claude Opus 5 --- spec/src/modules/search.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/src/modules/search.js b/spec/src/modules/search.js index 7363726f..4a60dc0b 100644 --- a/spec/src/modules/search.js +++ b/spec/src/modules/search.js @@ -494,7 +494,7 @@ describe('ConstructorIO - Search', () => { expect(res.response.results[0].data.facets.find((facet) => facet.name === 'Color').values).to.be.an('array').that.include('red'); expect(res.response.results[1].data.facets.find((facet) => facet.name === 'Color').values).to.be.an('array').that.include('blue'); done(); - }); + }).catch(done); }); it('Should return a response with a valid query, section, and qs param', (done) => {