diff options
author | Robert Nagy <ronagy@icloud.com> | 2019-06-07 14:20:43 +0200 |
---|---|---|
committer | Rich Trott <rtrott@gmail.com> | 2019-07-14 21:48:21 -0700 |
commit | 461bf36d701f3f7c669e2d916d5a5bc17fc447bf (patch) | |
tree | de2d9b1288388fd24591210d904af0b9ea14be09 | |
parent | 2550ddb04909f19068e947a18681e6c675346e42 (diff) | |
download | android-node-v8-461bf36d701f3f7c669e2d916d5a5bc17fc447bf.tar.gz android-node-v8-461bf36d701f3f7c669e2d916d5a5bc17fc447bf.tar.bz2 android-node-v8-461bf36d701f3f7c669e2d916d5a5bc17fc447bf.zip |
http: fix test where aborted should not be emitted
PR-URL: https://github.com/nodejs/node/pull/20077
Fixes: https://github.com/nodejs/node/issues/20107
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
-rw-r--r-- | doc/api/http.md | 5 | ||||
-rw-r--r-- | lib/_http_client.js | 16 | ||||
-rw-r--r-- | test/parallel/test-http-client-aborted.js | 23 | ||||
-rw-r--r-- | test/parallel/test-http-client-no-error-after-aborted.js | 21 | ||||
-rw-r--r-- | test/parallel/test-http-client-timeout-on-connect.js | 3 | ||||
-rw-r--r-- | test/parallel/test-http-writable-true-after-close.js | 2 |
6 files changed, 60 insertions, 10 deletions
diff --git a/doc/api/http.md b/doc/api/http.md index cc0c7ae1ce..e9ed20f05f 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -541,7 +541,8 @@ added: v0.3.8 --> Marks the request as aborting. Calling this will cause remaining data -in the response to be dropped and the socket to be destroyed. +in the response to be dropped and the socket to be destroyed. After +calling this method no further errors will be emitted. ### request.aborted <!-- YAML @@ -2142,8 +2143,6 @@ will be emitted in the following order: * `'socket'` * (`req.abort()` called here) * `'abort'` -* `'error'` with an error with message `'Error: socket hang up'` and code - `'ECONNRESET'` * `'close'` If `req.abort()` is called after the response is received, the following events diff --git a/lib/_http_client.js b/lib/_http_client.js index a1750a1a00..f2282e207d 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -374,7 +374,9 @@ function socketCloseListener() { // receive a response. The error needs to // fire on the request. req.socket._hadError = true; - req.emit('error', connResetException('socket hang up')); + if (!req.aborted) { + req.emit('error', connResetException('socket hang up')); + } } req.emit('close'); } @@ -400,7 +402,9 @@ function socketErrorListener(err) { // For Safety. Some additional errors might fire later on // and we need to make sure we don't double-fire the error event. req.socket._hadError = true; - req.emit('error', err); + if (!req.aborted) { + req.emit('error', err); + } } // Handle any pending data @@ -434,7 +438,9 @@ function socketOnEnd() { // If we don't have a response then we know that the socket // ended prematurely and we need to emit an error on the request. req.socket._hadError = true; - req.emit('error', connResetException('socket hang up')); + if (!req.aborted) { + req.emit('error', connResetException('socket hang up')); + } } if (parser) { parser.finish(); @@ -457,7 +463,9 @@ function socketOnData(d) { freeParser(parser, req, socket); socket.destroy(); req.socket._hadError = true; - req.emit('error', ret); + if (!req.aborted) { + req.emit('error', ret); + } } else if (parser.incoming && parser.incoming.upgrade) { // Upgrade (if status code 101) or CONNECT var bytesParsed = ret; diff --git a/test/parallel/test-http-client-aborted.js b/test/parallel/test-http-client-aborted.js new file mode 100644 index 0000000000..58e8d589c7 --- /dev/null +++ b/test/parallel/test-http-client-aborted.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require('../common'); +const http = require('http'); +const assert = require('assert'); + +const server = http.createServer(common.mustCall(function(req, res) { + req.on('aborted', common.mustCall(function() { + assert.strictEqual(this.aborted, true); + server.close(); + })); + assert.strictEqual(req.aborted, false); + res.write('hello'); +})); + +server.listen(0, common.mustCall(() => { + const req = http.get({ + port: server.address().port, + headers: { connection: 'keep-alive' } + }, common.mustCall((res) => { + req.abort(); + })); +})); diff --git a/test/parallel/test-http-client-no-error-after-aborted.js b/test/parallel/test-http-client-no-error-after-aborted.js new file mode 100644 index 0000000000..21f72008a9 --- /dev/null +++ b/test/parallel/test-http-client-no-error-after-aborted.js @@ -0,0 +1,21 @@ +'use strict'; + +const common = require('../common'); +const http = require('http'); + +const server = http.createServer(common.mustCall((req, res) => { + res.write('hello'); +})); + +server.listen(0, common.mustCall(() => { + const req = http.get({ + port: server.address().port + }, common.mustCall((res) => { + req.on('error', common.mustNotCall()); + req.abort(); + req.socket.destroy(new Error()); + req.on('close', common.mustCall(() => { + server.close(); + })); + })); +})); diff --git a/test/parallel/test-http-client-timeout-on-connect.js b/test/parallel/test-http-client-timeout-on-connect.js index 3a0098229d..d2c6975e25 100644 --- a/test/parallel/test-http-client-timeout-on-connect.js +++ b/test/parallel/test-http-client-timeout-on-connect.js @@ -23,8 +23,7 @@ server.listen(0, common.localhostIPv4, common.mustCall(() => { })); })); req.on('timeout', common.mustCall(() => req.abort())); - req.on('error', common.mustCall((err) => { - assert.strictEqual(err.message, 'socket hang up'); + req.on('abort', common.mustCall(() => { server.close(); })); })); diff --git a/test/parallel/test-http-writable-true-after-close.js b/test/parallel/test-http-writable-true-after-close.js index c0db7c3449..1174eec2ec 100644 --- a/test/parallel/test-http-writable-true-after-close.js +++ b/test/parallel/test-http-writable-true-after-close.js @@ -34,7 +34,7 @@ const server = createServer(common.mustCall((req, res) => { })); }).listen(0, () => { external = get(`http://127.0.0.1:${server.address().port}`); - external.on('error', common.mustCall(() => { + external.on('abort', common.mustCall(() => { server.close(); internal.close(); })); |