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/spec/fixtures/catalog.tar.gz b/spec/fixtures/catalog.tar.gz new file mode 100644 index 00000000..4d36d0d4 Binary files /dev/null and b/spec/fixtures/catalog.tar.gz differ 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/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) => { 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', }; 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,