From ac2140cb4b768a1023c1ba295c46500e1263d56d Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:35:57 +0100 Subject: [PATCH 1/8] refactor!: Replace http.request by fetch --- index.js | 75 +++------------ package.json | 2 +- test/datasource-solr.spec.js | 178 ++++++++++++----------------------- 3 files changed, 74 insertions(+), 181 deletions(-) diff --git a/index.js b/index.js index ff6bba4..d79ad67 100644 --- a/index.js +++ b/index.js @@ -1,6 +1,5 @@ 'use strict'; -const http = require('http'); const querystring = require('querystring'); const { ImplementationError } = require('@florajs/errors'); @@ -185,14 +184,6 @@ function buildSolrOrderString(floraOrders) { return floraOrders.map((order) => order.attribute + ' ' + order.direction).join(','); } -function parseData(str) { - try { - return JSON.parse(str); - } catch (e) { - return new Error('Could not parse response: ' + str); - } -} - function prepareQueryAddition(queryAdditions) { return queryAdditions .replace(/[\r\n]+/g, ' ') @@ -224,50 +215,24 @@ function getUrlGenerators(servers) { * * @param {string} requestUrl * @param {Object} params - * @param {Object} requestOptions - * @param {Agent} agent + * @param {number} timeout * @returns {Promise} * @private */ -function querySolr(requestUrl, params, requestOptions, agent) { - return new Promise((resolve, reject) => { - const options = { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - timeout: requestOptions.connectTimeout, - agent - }; - - const req = http.request(requestUrl, options, (res) => { - const chunks = []; - - res.on('data', (chunk) => chunks.push(chunk)); - - res.on('end', () => { - const data = parseData(Buffer.concat(chunks).toString('utf8')); - - if (res.statusCode >= 400 || data instanceof Error) { - const error = new Error(`Solr error: ${res.statusCode} ${http.STATUS_CODES[res.statusCode]}`); - return reject(error); - } - - return resolve({ totalCount: data.response.numFound, data: data.response.docs }); - }); - }); - - req.write(querystring.stringify(params)); // add params to POST body +async function querySolr(requestUrl, params, timeout) { + const response = await fetch(requestUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: querystring.stringify(params), + signal: AbortSignal.timeout(timeout) + }); - req.on('error', (err) => { - err.message = `Solr error: ${err.message} (${options.host})`; - reject(err); - }); - req.on('timeout', () => { - req.destroy(); - reject(new Error('Request timed out')); - }); + if (!response.ok) { + throw new Error(`Solr error: ${response.status} - ${response.statusText}`); + } - req.end(); - }); + const { numFound, docs } = (await response.json()).response; + return { totalCount: numFound, data: docs }; } function prepareSearchTerm(request) { @@ -301,12 +266,6 @@ class DataSource { this._urls = getUrlGenerators(config.servers); this._status = config._status; delete config._status; - - this._agent = new http.Agent({ - maxSockets: 5, - keepAlive: true, - keepAliveMsecs: 10000 - }); } /** @@ -360,12 +319,8 @@ class DataSource { if (request._explain) Object.assign(request._explain, { url: requestUrl, params }); if (this._status) this._status.increment('dataSourceQueries'); - const requestOpts = { - connectTimeout: serverOpts[server].connectTimeout || 2000, - requestTimeout: serverOpts[server].requestTimeout || 5000 - }; - - return querySolr(requestUrl, params, requestOpts, this._agent); + const timeout = serverOpts[server].timeout || 5000; + return querySolr(requestUrl, params, timeout); } /** diff --git a/package.json b/package.json index 780709c..ec838fa 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "eslint-config-prettier": "^9.1.0", "eslint-plugin-prettier": "^5.2.1", "globals": "^15.13.0", - "nock": "^13.5.6", + "nock": "^14.0.10", "prettier": "^3.0.1" } } diff --git a/test/datasource-solr.spec.js b/test/datasource-solr.spec.js index 534f9fd..743e586 100644 --- a/test/datasource-solr.spec.js +++ b/test/datasource-solr.spec.js @@ -64,6 +64,15 @@ describe('Flora SOLR DataSource', () => { assert.ok(scope.isDone()); }); + it('should parse Solr response', async () => { + const scope = nock(solrUrl).post(solrIndexPath).reply(200, testResponse); + + const response = await dataSource.process({ collection: 'article' }); + + assert.deepEqual(response, { totalCount: 0, data: [] }); + assert.ok(scope.isDone()); + }); + it('should use "default" if no explicit server is specified', async () => { const scope = nock(solrUrl).post(solrIndexPath).reply(200, testResponse); @@ -90,64 +99,38 @@ describe('Flora SOLR DataSource', () => { assert.ok(otherScope.isDone()); }); - describe('error handling', () => { + describe('error handling', async () => { it('should trigger error if status code >= 400', async () => { nock(solrUrl).post(solrIndexPath).reply(500, '{}'); - try { - await dataSource.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'message')); - assert.ok(e.message.includes('500')); - return; - } - - assert.fail('Expected request to fail'); + await assert.rejects(() => dataSource.process({ collection: 'article' }), { + name: 'Error', + message: /\b500\b/ + }); }); it('should trigger error if response cannot be parsed', async () => { nock(solrUrl).post(solrIndexPath).reply(418, '

Something went wrong

'); - try { - await dataSource.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'message')); - assert.ok(e.message.includes(`I'm a Teapot`)); - return; - } - - assert.fail('Expected request to fail'); - }); - - it.skip("should handle request's error event", async () => { - // nock can't fake request errors at the moment, so we have to make a real request to nonexistent host - dataSource = new FloraSolr(api, { - servers: { - default: { urls: ['http://doesnotexists.localhost/solr/'] } + await assert.rejects( + () => dataSource.process({ collection: 'article' }), + (err) => { + assert.equal(err.name, 'Error'); + assert.ok(err.message.includes(`I'm a Teapot`)); + return true; } - }); - - try { - await dataSource.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'code')); - assert.equal(e.code, 'ENOTFOUND'); - return; - } - - assert.fail('Expected request to fail'); + ); }); it('should trigger an error for non-existent server', async () => { - try { - await dataSource.process({ server: 'non-existent', collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'message')); - assert.ok(e.message.includes('Server "non-existent" not defined')); - return; - } - - assert.fail('Expected request to fail'); + await assert.rejects( + () => dataSource.process({ server: 'non-existent', collection: 'article' }), + (err) => { + assert.equal(err.name, 'Error'); + assert.ok(err.message.includes('Server "non-existent" not defined')); + return true; + } + ); }); }); @@ -608,87 +591,42 @@ describe('Flora SOLR DataSource', () => { }); }); - // https://github.com/nock/nock/issues/506 - describe.skip('timeout', () => { + // https://github.com/nodejs/node/issues/60509 + describe.skip('timeouts', () => { afterEach(() => nock.abortPendingRequests()); - describe('defaults', () => { - it('should set connect timeout', async () => { - nock(solrUrl).post(solrIndexPath).delayConnection(15000).reply(200, testResponse); - - try { - await dataSource.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'code')); - assert.equal(e.code, 'ETIMEDOUT'); - return; - } - - assert.fail('Expected request to fail'); - }); - - it('should set request timeout to 10 seconds', async () => { - nock(solrUrl).post(solrIndexPath).delayBody(11000).reply(200, testResponse); - - try { - await dataSource.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'code')); - assert.equal(e.code, 'ECONNRESET'); - return; - } - - assert.fail('Expected request to fail'); - }); - }); - - describe('config options', () => { - it('should overwrite default connect timeout', async () => { - const ds = new FloraSolr(api, { + Object.entries({ + 'should set default timeout to 5 seconds': [ + new FloraSolr(api, { servers: { - default: { - urls: ['http://example.com/solr/'], - connectTimeout: 100 - } + default: { urls: ['http://example.com/solr/'] } } - }); - - nock(solrUrl).post(solrIndexPath).delayConnection(200).reply(200, testResponse); - - try { - await ds.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'code')); - assert.equal(e.code, 'ETIMEDOUT'); - return; - } - - assert.fail('Expected request to fail'); - }); - - it.skip('should overwrite default request timeout', async () => { - const ds = new FloraSolr(api, { + }), + 6000 + ], + 'should overwrite (default) timeout': [ + new FloraSolr(api, { servers: { - default: { - urls: ['http://example.com/solr/'], - requestTimeout: 100 - } + default: { urls: ['http://example.com/solr/'], timeout: 100 } } + }), + 1000 + ] + }).forEach(([description, [ds, timeoutMs]]) => + it(description, async (ctx) => { + ctx.mock.timers.enable(); + nock(solrUrl).post(solrIndexPath).delay(timeoutMs).reply(200, testResponse); + + const promise = ds.process({ collection: 'article' }); + ctx.mock.timers.tick(timeoutMs); + + await assert.rejects(promise, { + name: 'TimeoutError', + message: 'The operation was aborted due to timeout' }); - - nock(solrUrl).post(solrIndexPath).delayBody(200).reply(200, testResponse); - - try { - await ds.process({ collection: 'article' }); - } catch (e) { - assert.ok(Object.hasOwn(e, 'code')); - assert.equal(e.code, 'ECONNRESET'); - return; - } - - assert.fail('Expected request to fail'); - }); - }); + ctx.mock.timers.reset(); + }) + ); }); describe('limitPer', () => { From 0ae6a71a11c2e49fc83a11832cf5e06bd684c55e Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:37:38 +0100 Subject: [PATCH 2/8] build: Update dev dependencies --- package.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index ec838fa..dc2889c 100644 --- a/package.json +++ b/package.json @@ -43,11 +43,11 @@ "@florajs/errors": "^4.0.0" }, "devDependencies": { - "eslint": "^9.16.0", - "eslint-config-prettier": "^9.1.0", - "eslint-plugin-prettier": "^5.2.1", - "globals": "^15.13.0", + "eslint": "^9.39.1", + "eslint-config-prettier": "^10.1.8", + "eslint-plugin-prettier": "^5.5.4", + "globals": "^16.5.0", "nock": "^14.0.10", - "prettier": "^3.0.1" + "prettier": "^3.6.2" } } From 583e0d993ccc05cc99c4739ffa04f623083e9ad3 Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:41:39 +0100 Subject: [PATCH 3/8] ci!: Remove support for Node.js 18 --- .github/workflows/ci.yml | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1648ecc..981969f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node: ['18', '20', '22'] + node: ['20', '22'] steps: - uses: actions/checkout@v4 - name: Setup node ${{ matrix.node }} diff --git a/package.json b/package.json index dc2889c..d5e3468 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ } ], "engines": { - "node": ">=18" + "node": ">=20" }, "dependencies": { "@florajs/errors": "^4.0.0" From c8ffac67a8c246fe5182b99ee8375b912995772f Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:43:03 +0100 Subject: [PATCH 4/8] ci: Run tests using Node.js 24 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 981969f..6c9e615 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node: ['20', '22'] + node: ['20', '22', '24'] steps: - uses: actions/checkout@v4 - name: Setup node ${{ matrix.node }} From a4226421ff886949ed67a466ec1096ac8733b2ab Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:48:12 +0100 Subject: [PATCH 5/8] ci: Run pipeline for pull requests --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c9e615..8781280 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,10 @@ name: ci on: + pull_request: + types: + - opened + - synchronize push: branches: - main From b5b0f9b347d5f3b32085ebdb72547415f2bd46f8 Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:55:51 +0100 Subject: [PATCH 6/8] ci: Update actions/checkout --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8781280..eea8b2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: matrix: node: ['20', '22', '24'] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Setup node ${{ matrix.node }} uses: actions/setup-node@v4 with: From c550cd73da792cfd0a0fb5c33ef2b6f9b9b67367 Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 16:58:06 +0100 Subject: [PATCH 7/8] ci: Update actions/setup-node --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eea8b2f..25dedf6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: steps: - uses: actions/checkout@v5 - name: Setup node ${{ matrix.node }} - uses: actions/setup-node@v4 + uses: actions/setup-node@v6 with: node-version: ${{ matrix.node }} - run: npm install From 327febba42874c0c79c179a36d66171eeb401a50 Mon Sep 17 00:00:00 2001 From: Marco Baumgartl Date: Tue, 4 Nov 2025 17:21:35 +0100 Subject: [PATCH 8/8] refactor!: Replace legacy querystring module by URLSearchParams --- index.js | 4 +--- test/datasource-solr.spec.js | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index d79ad67..2608177 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,5 @@ 'use strict'; -const querystring = require('querystring'); - const { ImplementationError } = require('@florajs/errors'); const SUPPORTED_FILTERS = ['equal', 'notEqual', 'less', 'lessOrEqual', 'greater', 'greaterOrEqual', 'range']; @@ -223,7 +221,7 @@ async function querySolr(requestUrl, params, timeout) { const response = await fetch(requestUrl, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: querystring.stringify(params), + body: new URLSearchParams(params).toString(), signal: AbortSignal.timeout(timeout) }); diff --git a/test/datasource-solr.spec.js b/test/datasource-solr.spec.js index 743e586..0f3ef92 100644 --- a/test/datasource-solr.spec.js +++ b/test/datasource-solr.spec.js @@ -517,7 +517,7 @@ describe('Flora SOLR DataSource', () => { }; const scope = nock(solrUrl) - .post(solrIndexPath, /sort=foo%20asc/) + .post(solrIndexPath, ({ sort }) => sort === 'foo asc') .reply(200, testResponse); await dataSource.process(request); @@ -535,7 +535,7 @@ describe('Flora SOLR DataSource', () => { }; const scope = nock(solrUrl) - .post(solrIndexPath, /sort=foo%20asc%2Cbar%20desc/) + .post(solrIndexPath, ({ sort }) => sort === 'foo asc,bar desc') .reply(200, testResponse); await dataSource.process(request);