diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1648ecc..25dedf6 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 @@ -13,11 +17,11 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node: ['18', '20', '22'] + node: ['20', '22', '24'] steps: - - uses: actions/checkout@v4 + - 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 diff --git a/index.js b/index.js index ff6bba4..2608177 100644 --- a/index.js +++ b/index.js @@ -1,8 +1,5 @@ 'use strict'; -const http = require('http'); -const querystring = require('querystring'); - const { ImplementationError } = require('@florajs/errors'); const SUPPORTED_FILTERS = ['equal', 'notEqual', 'less', 'lessOrEqual', 'greater', 'greaterOrEqual', 'range']; @@ -185,14 +182,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 +213,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: new URLSearchParams(params).toString(), + 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 +264,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 +317,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..d5e3468 100644 --- a/package.json +++ b/package.json @@ -37,17 +37,17 @@ } ], "engines": { - "node": ">=18" + "node": ">=20" }, "dependencies": { "@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", - "nock": "^13.5.6", - "prettier": "^3.0.1" + "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.6.2" } } diff --git a/test/datasource-solr.spec.js b/test/datasource-solr.spec.js index 534f9fd..0f3ef92 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; + } + ); }); }); @@ -534,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); @@ -552,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); @@ -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', () => {