From 049d7833e56455d79f3dda129e1118f31511d5d3 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Fri, 25 Nov 2022 00:55:17 +0100 Subject: [PATCH 1/7] Add abort signal support --- README.md | 2 + lib/needle.js | 33 ++++++++++-- test/errors_spec.js | 125 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1adff8972..4e29bca74 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,7 @@ With only two real dependencies, Needle supports: - Automatic XML & JSON parsing - 301/302/303 redirect following, with fine-grained tuning, and - Streaming non-UTF-8 charset decoding, via `iconv-lite` + - Aborting any or all Needle requests using `AbortSignal` objects And yes, Mr. Wayne, it does come in black. @@ -317,6 +318,7 @@ For information about options that've changed, there's always [the changelog](ht - `stream_length`: When sending streams, this lets you manually set the Content-Length header --if the stream's bytecount is known beforehand--, preventing ECONNRESET (socket hang up) errors on some servers that misbehave when receiving payloads of unknown size. Set it to `0` and Needle will get and set the stream's length for you, or leave unset for the default behaviour, which is no Content-Length header for stream payloads. - `localAddress`: , IP address. Passed to http/https request. Local interface from which the request should be emitted. - `uri_modifier`: Anonymous function taking request (or redirect location if following redirects) URI as an argument and modifying it given logic. It has to return a valid URI string for successful request. + - `signal` : An `AbortSignal` object that can be used to abort any or all Needle requests. Response options ---------------- diff --git a/lib/needle.js b/lib/needle.js index df5f1264f..56372cf69 100644 --- a/lib/needle.js +++ b/lib/needle.js @@ -97,6 +97,9 @@ var defaults = { follow_max : 0, stream_length : -1, + // abort signal + signal : null, + // booleans compressed : false, decode_response : true, @@ -188,7 +191,8 @@ Needle.prototype.setup = function(uri, options) { http_opts : { agent: get_option('agent', defaults.agent), localAddress: get_option('localAddress', undefined), - lookup: get_option('lookup', undefined) + lookup: get_option('lookup', undefined), + signal: get_option('signal', defaults.signal) }, // passed later to http.request() directly headers : {}, output : options.output, @@ -205,6 +209,9 @@ Needle.prototype.setup = function(uri, options) { config[key] = check_value('number', key); }) + if (config.http_opts.signal && !(config.http_opts.signal instanceof AbortSignal)) + throw new TypeError(typeof config.http_opts.signal + ' received for signal, but expected an AbortSignal'); + // populate http_opts with given TLS options tls_options.split(' ').forEach(function(key) { if (typeof options[key] != 'undefined') { @@ -443,6 +450,7 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, self = this, request_opts = this.get_request_opts(method, uri, config), protocol = request_opts.protocol == 'https:' ? https : http; + signal = request_opts.signal; function done(err, resp) { if (returned++ > 0) @@ -478,6 +486,11 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, done(err || new Error('Unknown error when making request.')); } + function abort_handler() { + out.emit('err', new Error('Aborted by signal.')); + request.abort(); + } + function set_timeout(type, milisecs) { if (timer) clearTimeout(timer); if (milisecs <= 0) return; @@ -487,10 +500,11 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, request.abort(); // also invoke done() to terminate job on read_timeout if (type == 'read') done(new Error(type + ' timeout')); + + signal && signal.removeEventListener('abort', abort_handler); }, milisecs); } - debug('Making request #' + count, request_opts); request = protocol.request(request_opts, function(resp) { @@ -763,6 +777,15 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, request.end(); } + // Manage the abort signal + if ( signal ) { + if ( signal.aborted === true ) { + abort_handler(); + } else { + signal.addEventListener('abort', abort_handler, { once: true }); + } + } + out.abort = function() { request.abort() }; // easier access out.request = request; return out; @@ -798,12 +821,14 @@ module.exports.defaults = function(obj) { var target_key = aliased.options[key] || key; if (defaults.hasOwnProperty(target_key) && typeof obj[key] != 'undefined') { - if (target_key != 'parse_response' && target_key != 'proxy' && target_key != 'agent') { - // ensure type matches the original, except for proxy/parse_response that can be null/bool or string + if (target_key != 'parse_response' && target_key != 'proxy' && target_key != 'agent' && target_key != 'signal') { + // ensure type matches the original, except for proxy/parse_response that can be null/bool or string, and signal that can be null/AbortSignal var valid_type = defaults[target_key].constructor.name; if (obj[key].constructor.name != valid_type) throw new TypeError('Invalid type for ' + key + ', should be ' + valid_type); + } else if (target_key === 'signal' && obj[key] !== null && !(obj[key] instanceof AbortSignal)) { + throw new TypeError('Invalid type for ' + key + ', should be AbortSignal'); } defaults[target_key] = obj[key]; } else { diff --git a/test/errors_spec.js b/test/errors_spec.js index c19568144..e04691b4a 100644 --- a/test/errors_spec.js +++ b/test/errors_spec.js @@ -269,4 +269,129 @@ describe('errors', function() { }) + describe('when request is aborted by signal', function() { + + var server, + url = 'http://localhost:3333/foo'; + + before(function() { + server = helpers.server({ port: 3333, wait: 600 }); + }) + + after(function() { + server.close(); + }) + + afterEach(function() { + // reset signal to default + needle.defaults({signal: null}); + }) + + it('works if passing an already aborted signal aborts the request', function(done) { + const abortedSignal = AbortSignal.abort(); + const start = new Date(); + + abortedSignal.aborted.should.equal(true); + + needle.get(url, { signal: abortedSignal, response_timeout: 10000 }, function(err, res) { + const timediff = (new Date() - start); + + should.not.exist(res); + err.code.should.equal('ABORT_ERR'); + timediff.should.be.within(0, 50); + + done(); + }); + }) + + it('works if request aborts before timing out', function(done) { + const cancel = new AbortController(); + const start = new Date(); + + needle.get(url, { signal: cancel.signal, response_timeout: 500, open_timeout: 500, read_timeout: 500 }, function(err, res) { + const timediff = (new Date() - start); + + should.not.exist(res); + err.code.should.equal('ECONNRESET'); + cancel.signal.aborted.should.equal(true); + timediff.should.be.within(200, 250); + + done(); + }); + + function abort() { + cancel.abort(); + } + setTimeout(abort, 200); + }) + + it('works if request times out before being aborted', function(done) { + const cancel = new AbortController(); + const start = new Date(); + + needle.get(url, { signal: cancel.signal, response_timeout: 200, open_timeout: 200, read_timeout: 200 }, function(err, res) { + const timediff = (new Date() - start); + + should.not.exist(res); + err.code.should.equal('ECONNRESET'); + timediff.should.be.within(200, 250); + }); + + function abort() { + cancel.signal.aborted.should.equal(false); + done(); + } + setTimeout(abort, 500); + }) + + it('works if setting default signal aborts all requests', function(done) { + const cancel = new AbortController(); + + needle.defaults({signal: cancel.signal}); + + const start = new Date(); + let count = 0; + function cb(err, res) { + const timediff = (new Date() - start); + + should.not.exist(res); + err.code.should.equal('ECONNRESET'); + cancel.signal.aborted.should.equal(true); + timediff.should.be.within(200, 250); + + if ( count++ === 2 ) done(); + } + + needle.get(url, { timeout: 300 }, cb); + needle.get(url, { timeout: 350 }, cb); + needle.get(url, { timeout: 400}, cb); + + function abort() { + cancel.abort(); + } + setTimeout(abort, 200); + }) + + it('does not work if invalid signal passed', function(done) { + try { + needle.get(url, { signal: 'invalid signal' }, function(err, res) { + done(new Error('A bad option error expected to be thrown')); + }); + } catch(e) { + e.should.be.a.TypeError; + done(); + } + }) + + it('does not work if invalid signal set by default', function(done) { + try { + needle.defaults({signal: new Error(), timeout: 1200}); + done(new Error('A bad option error expected to be thrown')); + } catch(e) { + e.should.be.a.TypeError; + done(); + } + }) + + }) }) From d0d6fca953eedf1b006366f62176409d045e43d5 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Fri, 25 Nov 2022 14:56:35 +0100 Subject: [PATCH 2/7] Replace deprecated abort with destroy --- lib/needle.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/needle.js b/lib/needle.js index 56372cf69..70ce8ce70 100644 --- a/lib/needle.js +++ b/lib/needle.js @@ -488,7 +488,7 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, function abort_handler() { out.emit('err', new Error('Aborted by signal.')); - request.abort(); + request.destroy(); } function set_timeout(type, milisecs) { @@ -497,7 +497,7 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, timer = setTimeout(function() { out.emit('timeout', type); - request.abort(); + request.destroy(); // also invoke done() to terminate job on read_timeout if (type == 'read') done(new Error(type + ' timeout')); @@ -786,7 +786,7 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, } } - out.abort = function() { request.abort() }; // easier access + out.abort = function() { request.destroy() }; // easier access out.request = request; return out; } From 9d9242bc81dc07d99addae9fe27d433bfcda1062 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Fri, 25 Nov 2022 17:23:53 +0100 Subject: [PATCH 3/7] Conform to current style --- lib/needle.js | 6 +++--- test/errors_spec.js | 28 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/needle.js b/lib/needle.js index 70ce8ce70..91e8f4d1a 100644 --- a/lib/needle.js +++ b/lib/needle.js @@ -778,11 +778,11 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data, } // Manage the abort signal - if ( signal ) { - if ( signal.aborted === true ) { + if (signal) { + if (signal.aborted === true) { abort_handler(); } else { - signal.addEventListener('abort', abort_handler, { once: true }); + signal.addEventListener('abort', abort_handler, {once: true}); } } diff --git a/test/errors_spec.js b/test/errors_spec.js index e04691b4a..d28b87130 100644 --- a/test/errors_spec.js +++ b/test/errors_spec.js @@ -269,7 +269,7 @@ describe('errors', function() { }) - describe('when request is aborted by signal', function() { + describe.only('when request is aborted by signal', function() { var server, url = 'http://localhost:3333/foo'; @@ -288,13 +288,13 @@ describe('errors', function() { }) it('works if passing an already aborted signal aborts the request', function(done) { - const abortedSignal = AbortSignal.abort(); - const start = new Date(); + var abortedSignal = AbortSignal.abort(); + var start = new Date(); abortedSignal.aborted.should.equal(true); needle.get(url, { signal: abortedSignal, response_timeout: 10000 }, function(err, res) { - const timediff = (new Date() - start); + var timediff = (new Date() - start); should.not.exist(res); err.code.should.equal('ABORT_ERR'); @@ -305,11 +305,11 @@ describe('errors', function() { }) it('works if request aborts before timing out', function(done) { - const cancel = new AbortController(); - const start = new Date(); + var cancel = new AbortController(); + var start = new Date(); needle.get(url, { signal: cancel.signal, response_timeout: 500, open_timeout: 500, read_timeout: 500 }, function(err, res) { - const timediff = (new Date() - start); + var timediff = (new Date() - start); should.not.exist(res); err.code.should.equal('ECONNRESET'); @@ -326,11 +326,11 @@ describe('errors', function() { }) it('works if request times out before being aborted', function(done) { - const cancel = new AbortController(); - const start = new Date(); + var cancel = new AbortController(); + var start = new Date(); needle.get(url, { signal: cancel.signal, response_timeout: 200, open_timeout: 200, read_timeout: 200 }, function(err, res) { - const timediff = (new Date() - start); + var timediff = (new Date() - start); should.not.exist(res); err.code.should.equal('ECONNRESET'); @@ -345,14 +345,14 @@ describe('errors', function() { }) it('works if setting default signal aborts all requests', function(done) { - const cancel = new AbortController(); + var cancel = new AbortController(); needle.defaults({signal: cancel.signal}); - const start = new Date(); - let count = 0; + var start = new Date(); + var count = 0; function cb(err, res) { - const timediff = (new Date() - start); + var timediff = (new Date() - start); should.not.exist(res); err.code.should.equal('ECONNRESET'); From 252e5c009191008279ae09ce11e4f45665a65f46 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Fri, 25 Nov 2022 17:27:29 +0100 Subject: [PATCH 4/7] Remove only from ut --- test/errors_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/errors_spec.js b/test/errors_spec.js index d28b87130..d7471849b 100644 --- a/test/errors_spec.js +++ b/test/errors_spec.js @@ -269,7 +269,7 @@ describe('errors', function() { }) - describe.only('when request is aborted by signal', function() { + describe('when request is aborted by signal', function() { var server, url = 'http://localhost:3333/foo'; From a35b2bd2f73744eec3372c243e10461b03e1c4e8 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Fri, 25 Nov 2022 23:50:02 +0100 Subject: [PATCH 5/7] update ut to make them run with node >= 16 --- test/errors_spec.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/errors_spec.js b/test/errors_spec.js index d7471849b..667679223 100644 --- a/test/errors_spec.js +++ b/test/errors_spec.js @@ -312,7 +312,8 @@ describe('errors', function() { var timediff = (new Date() - start); should.not.exist(res); - err.code.should.equal('ECONNRESET'); + // err.code.should.equal('ECONNRESET'); works with node 16, not with node > 16 + err.code.should.match(/ECONNRESET|ABORT_ERR/) // Work with node >= 16 cancel.signal.aborted.should.equal(true); timediff.should.be.within(200, 250); @@ -355,7 +356,8 @@ describe('errors', function() { var timediff = (new Date() - start); should.not.exist(res); - err.code.should.equal('ECONNRESET'); + // err.code.should.equal('ECONNRESET'); works with node 16, not with node > 16 + err.code.should.match(/ECONNRESET|ABORT_ERR/) // Work with node >= 16 cancel.signal.aborted.should.equal(true); timediff.should.be.within(200, 250); From 12d272ac752bcc15f0ec7d77421542eb32888e39 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Fri, 25 Nov 2022 23:57:11 +0100 Subject: [PATCH 6/7] better manage versions --- test/errors_spec.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/errors_spec.js b/test/errors_spec.js index 667679223..eae8db7ec 100644 --- a/test/errors_spec.js +++ b/test/errors_spec.js @@ -272,7 +272,8 @@ describe('errors', function() { describe('when request is aborted by signal', function() { var server, - url = 'http://localhost:3333/foo'; + url = 'http://localhost:3333/foo', + node_major_ver = process.version.split('.')[0].replace('v', ''); before(function() { server = helpers.server({ port: 3333, wait: 600 }); @@ -312,8 +313,10 @@ describe('errors', function() { var timediff = (new Date() - start); should.not.exist(res); - // err.code.should.equal('ECONNRESET'); works with node 16, not with node > 16 - err.code.should.match(/ECONNRESET|ABORT_ERR/) // Work with node >= 16 + if (node_major_ver <= 16) + err.code.should.equal('ECONNRESET'); + if (node_major_ver > 16) + err.code.should.equal('ABORT_ERR'); cancel.signal.aborted.should.equal(true); timediff.should.be.within(200, 250); @@ -356,8 +359,10 @@ describe('errors', function() { var timediff = (new Date() - start); should.not.exist(res); - // err.code.should.equal('ECONNRESET'); works with node 16, not with node > 16 - err.code.should.match(/ECONNRESET|ABORT_ERR/) // Work with node >= 16 + if (node_major_ver <= 16) + err.code.should.equal('ECONNRESET'); + if (node_major_ver > 16) + err.code.should.equal('ABORT_ERR'); cancel.signal.aborted.should.equal(true); timediff.should.be.within(200, 250); From 56178d5ce47c1f000e6e403ef80a1f4c143cdd98 Mon Sep 17 00:00:00 2001 From: Michele Crudele Date: Sat, 26 Nov 2022 00:06:46 +0100 Subject: [PATCH 7/7] run abort signal tests only for node >= 16 --- test/errors_spec.js | 258 ++++++++++++++++++++++---------------------- 1 file changed, 130 insertions(+), 128 deletions(-) diff --git a/test/errors_spec.js b/test/errors_spec.js index eae8db7ec..1c1cb04a2 100644 --- a/test/errors_spec.js +++ b/test/errors_spec.js @@ -269,136 +269,138 @@ describe('errors', function() { }) - describe('when request is aborted by signal', function() { - - var server, - url = 'http://localhost:3333/foo', - node_major_ver = process.version.split('.')[0].replace('v', ''); - - before(function() { - server = helpers.server({ port: 3333, wait: 600 }); - }) - - after(function() { - server.close(); - }) - - afterEach(function() { - // reset signal to default - needle.defaults({signal: null}); - }) - - it('works if passing an already aborted signal aborts the request', function(done) { - var abortedSignal = AbortSignal.abort(); - var start = new Date(); - - abortedSignal.aborted.should.equal(true); - - needle.get(url, { signal: abortedSignal, response_timeout: 10000 }, function(err, res) { - var timediff = (new Date() - start); - - should.not.exist(res); - err.code.should.equal('ABORT_ERR'); - timediff.should.be.within(0, 50); - - done(); - }); - }) - - it('works if request aborts before timing out', function(done) { - var cancel = new AbortController(); - var start = new Date(); - - needle.get(url, { signal: cancel.signal, response_timeout: 500, open_timeout: 500, read_timeout: 500 }, function(err, res) { - var timediff = (new Date() - start); - - should.not.exist(res); - if (node_major_ver <= 16) - err.code.should.equal('ECONNRESET'); - if (node_major_ver > 16) + var node_major_ver = process.version.split('.')[0].replace('v', ''); + if (node_major_ver >= 16) { + describe('when request is aborted by signal', function() { + + var server, + url = 'http://localhost:3333/foo'; + + before(function() { + server = helpers.server({ port: 3333, wait: 600 }); + }) + + after(function() { + server.close(); + }) + + afterEach(function() { + // reset signal to default + needle.defaults({signal: null}); + }) + + it('works if passing an already aborted signal aborts the request', function(done) { + var abortedSignal = AbortSignal.abort(); + var start = new Date(); + + abortedSignal.aborted.should.equal(true); + + needle.get(url, { signal: abortedSignal, response_timeout: 10000 }, function(err, res) { + var timediff = (new Date() - start); + + should.not.exist(res); err.code.should.equal('ABORT_ERR'); - cancel.signal.aborted.should.equal(true); - timediff.should.be.within(200, 250); - - done(); - }); - - function abort() { - cancel.abort(); - } - setTimeout(abort, 200); - }) - - it('works if request times out before being aborted', function(done) { - var cancel = new AbortController(); - var start = new Date(); - - needle.get(url, { signal: cancel.signal, response_timeout: 200, open_timeout: 200, read_timeout: 200 }, function(err, res) { - var timediff = (new Date() - start); - - should.not.exist(res); - err.code.should.equal('ECONNRESET'); - timediff.should.be.within(200, 250); - }); - - function abort() { - cancel.signal.aborted.should.equal(false); - done(); - } - setTimeout(abort, 500); - }) - - it('works if setting default signal aborts all requests', function(done) { - var cancel = new AbortController(); - - needle.defaults({signal: cancel.signal}); - - var start = new Date(); - var count = 0; - function cb(err, res) { - var timediff = (new Date() - start); - - should.not.exist(res); - if (node_major_ver <= 16) + timediff.should.be.within(0, 50); + + done(); + }); + }) + + it('works if request aborts before timing out', function(done) { + var cancel = new AbortController(); + var start = new Date(); + + needle.get(url, { signal: cancel.signal, response_timeout: 500, open_timeout: 500, read_timeout: 500 }, function(err, res) { + var timediff = (new Date() - start); + + should.not.exist(res); + if (node_major_ver <= 16) + err.code.should.equal('ECONNRESET'); + if (node_major_ver > 16) + err.code.should.equal('ABORT_ERR'); + cancel.signal.aborted.should.equal(true); + timediff.should.be.within(200, 250); + + done(); + }); + + function abort() { + cancel.abort(); + } + setTimeout(abort, 200); + }) + + it('works if request times out before being aborted', function(done) { + var cancel = new AbortController(); + var start = new Date(); + + needle.get(url, { signal: cancel.signal, response_timeout: 200, open_timeout: 200, read_timeout: 200 }, function(err, res) { + var timediff = (new Date() - start); + + should.not.exist(res); err.code.should.equal('ECONNRESET'); - if (node_major_ver > 16) - err.code.should.equal('ABORT_ERR'); - cancel.signal.aborted.should.equal(true); - timediff.should.be.within(200, 250); - - if ( count++ === 2 ) done(); - } - - needle.get(url, { timeout: 300 }, cb); - needle.get(url, { timeout: 350 }, cb); - needle.get(url, { timeout: 400}, cb); - - function abort() { - cancel.abort(); - } - setTimeout(abort, 200); - }) - - it('does not work if invalid signal passed', function(done) { - try { - needle.get(url, { signal: 'invalid signal' }, function(err, res) { - done(new Error('A bad option error expected to be thrown')); + timediff.should.be.within(200, 250); }); - } catch(e) { - e.should.be.a.TypeError; - done(); - } - }) - - it('does not work if invalid signal set by default', function(done) { - try { - needle.defaults({signal: new Error(), timeout: 1200}); - done(new Error('A bad option error expected to be thrown')); - } catch(e) { - e.should.be.a.TypeError; - done(); - } + + function abort() { + cancel.signal.aborted.should.equal(false); + done(); + } + setTimeout(abort, 500); + }) + + it('works if setting default signal aborts all requests', function(done) { + var cancel = new AbortController(); + + needle.defaults({signal: cancel.signal}); + + var start = new Date(); + var count = 0; + function cb(err, res) { + var timediff = (new Date() - start); + + should.not.exist(res); + if (node_major_ver <= 16) + err.code.should.equal('ECONNRESET'); + if (node_major_ver > 16) + err.code.should.equal('ABORT_ERR'); + cancel.signal.aborted.should.equal(true); + timediff.should.be.within(200, 250); + + if ( count++ === 2 ) done(); + } + + needle.get(url, { timeout: 300 }, cb); + needle.get(url, { timeout: 350 }, cb); + needle.get(url, { timeout: 400}, cb); + + function abort() { + cancel.abort(); + } + setTimeout(abort, 200); + }) + + it('does not work if invalid signal passed', function(done) { + try { + needle.get(url, { signal: 'invalid signal' }, function(err, res) { + done(new Error('A bad option error expected to be thrown')); + }); + } catch(e) { + e.should.be.a.TypeError; + done(); + } + }) + + it('does not work if invalid signal set by default', function(done) { + try { + needle.defaults({signal: new Error(), timeout: 1200}); + done(new Error('A bad option error expected to be thrown')); + } catch(e) { + e.should.be.a.TypeError; + done(); + } + }) + }) - - }) + } })