From 1e3a41f91b3c92fa333d05bb2f33cf2d94b3af7d Mon Sep 17 00:00:00 2001 From: Jelle De Loecker Date: Fri, 12 Apr 2024 14:46:33 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Don't=20throw=20errors=20when=20?= =?UTF-8?q?an=20*unused/free*=20socket=20has=20an=20error=20in=20`Develry.?= =?UTF-8?q?HttpAgent`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 + lib/request_agents.js | 31 ++++++-- test/request_agents.js | 159 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 185 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9e7047..ba4bf41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ * Add `Pledge#getResolvedFunction()` method, which can be passed as an old-style callback * Fix some `Swift` methods not using the correct schedulers * Implement a custom `done` and `waterfall` static method for the `SwiftPledge` class +* Make `Develry.HttpAgent` classes return sockets from `createConnection` +* Don't throw errors when an *unused/free* socket has an error in `Develry.HttpAgent` ## 0.9.2 (2024-02-25) diff --git a/lib/request_agents.js b/lib/request_agents.js index 2305ece..f157f38 100644 --- a/lib/request_agents.js +++ b/lib/request_agents.js @@ -457,7 +457,7 @@ HttpAgent.setMethod(function createSocketId() { * * @author Jelle De Loecker * @since 0.8.2 - * @version 0.8.2 + * @version 0.9.3 */ HttpAgent.setMethod(function initializeSocket(socket, options) { @@ -544,6 +544,15 @@ HttpAgent.setMethod(function initializeSocket(socket, options) { const on_error = (err) => { + const name = this.getName(options); + + // If the socket had an error while it was free, ignore the error + if (this.freeSockets[name] && this.freeSockets[name].indexOf(socket) !== -1) { + socket.destroy(); + this.removeSocket(socket, options); + return; + } + const listenerCount = socket.listeners('error').length; this.errorSocketCount++; @@ -575,7 +584,12 @@ HttpAgent.setMethod(function initializeSocket(socket, options) { * * @author Jelle De Loecker * @since 0.8.2 - * @version 0.8.2 + * @version 0.9.3 + * + * @param {Object} options + * @param {Function} on_create + * + * @return {Socket} */ HttpAgent.setMethod(function createConnection(options, on_create) { @@ -604,6 +618,8 @@ HttpAgent.setMethod(function createConnection(options, on_create) { if (new_socket) { on_new_create(null, new_socket); } + + return new_socket; }); /** @@ -664,11 +680,16 @@ const HttpsAgent = Fn.inherits('Develry.HttpAgent', function HttpsAgent(options) * * @author Jelle De Loecker * @since 0.8.2 - * @version 0.8.2 + * @version 0.9.3 + * + * @param {Object} options + * @param {Function} on_create + * + * @return {Socket} */ -HttpsAgent.setMethod(function createConnection(options) { +HttpsAgent.setMethod(function createConnection(options, on_create) { - const socket = CREATE_HTTPS_CONNECTION.call(this, options); + const socket = CREATE_HTTPS_CONNECTION.call(this, options, on_create); this.initializeSocket(socket, options); return socket; diff --git a/test/request_agents.js b/test/request_agents.js index 0a2fdfa..ac1090a 100644 --- a/test/request_agents.js +++ b/test/request_agents.js @@ -65,6 +65,11 @@ describe('HttpAgent', function() { recreateAgentKeepAlive(); app = http.createServer((req, res) => { + + const endResponse = (message) => { + res.end(message); + }; + if (req.url === '/error') { res.destroy(); return; @@ -81,12 +86,12 @@ describe('HttpAgent', function() { if (info.query.timeout) { setTimeout(() => { - res.end(info.query.timeout); + endResponse(info.query.timeout); }, parseInt(info.query.timeout)); return; } - res.end(JSON.stringify({ + endResponse(JSON.stringify({ info, url: req.url, headers: req.headers, @@ -2152,6 +2157,156 @@ describe('HttpsAgent', function() { }); }); + it('should support a lot of requests', async function () { + + this.timeout(30000); + + let agentkeepalive = new HttpsAgent({ + freeSocketTimeout: 100, + timeout: 20, + maxSockets: 30, + }); + + let count = 0; + + const capem = fs.readFileSync(__dirname + '/assets/ca.pem'); + + function makeRequest(i) { + + let pledge = new Blast.Classes.Pledge(); + + const req = https.get({ + agent: agentkeepalive, + headers: { + 'rcount' : '' + i, + }, + port, + path: '/?timeout=5', + ca: capem, + rejectUnauthorized: false, + }, res => { + + let data = ''; + + res.on('data', chunk => { + data += chunk; + }); + + res.on('end', () => { + count++; + pledge.resolve(data); + }); + }); + + req.on('error', err => { + pledge.reject(err); + }); + + return pledge; + } + + let promises = []; + + for (let i = 0; i < 300; i++) { + promises.push(makeRequest(i)); + + if (i % 5 == 0) { + await Pledge.after(6); + } + } + + await Pledge.all(promises); + + assert.strictEqual(count, 300); + }); + + it('should perform requests in series if there is only 1 socket to reuse', async function () { + + this.timeout(30000); + + let agentkeepalive = new HttpsAgent({ + freeSocketTimeout: 100, + timeout: 100, + maxSockets: 1, + }); + + let count = 0; + + const capem = fs.readFileSync(__dirname + '/assets/ca.pem'); + + let finished_indexes = []; + + function makeRequest(i, timeout) { + + let pledge = new Blast.Classes.Pledge(); + + if (!timeout) { + timeout = Number.random(1, 10); + } + + const req = https.get({ + agent: agentkeepalive, + headers: { + 'rcount' : '' + i, + }, + port, + path: '/?timeout=' + timeout, + ca: capem, + rejectUnauthorized: false, + }, res => { + + let data = ''; + + res.on('data', chunk => { + data += chunk; + }); + + res.on('end', () => { + finished_indexes.push(i); + count++; + pledge.resolve(data); + }); + }); + + req.on('error', err => { + pledge.reject(err); + }); + + return pledge; + } + + let promises = []; + + for (let i = 0; i < 30; i++) { + promises.push(makeRequest(i)); + } + + await Pledge.all(promises); + + assert.deepStrictEqual(finished_indexes, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29]); + + // Create an agent with more sockets + agentkeepalive = new HttpsAgent({ + freeSocketTimeout: 100, + timeout: 100, + maxSockets: 10, + }); + + promises = []; + finished_indexes = []; + + // Create requests with a fixed timeout + for (let i = 0; i < 30; i++) { + promises.push(makeRequest(i, 2)); + } + + await Pledge.all(promises); + + // This time, the requests should be done in parallel, + // and thus the indexes should be different + assert.notDeepStrictEqual(finished_indexes, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29]); + }); + it('should GET / success with 200 status', done => { https.get({ agent: agentkeepalive,