Skip to content

Commit

Permalink
http: fix test where aborted should not be emitted
Browse files Browse the repository at this point in the history
PR-URL: #20077
Fixes: #20107
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
ronag authored and Trott committed Jul 15, 2019
1 parent 2550ddb commit 461bf36
Showing 6 changed files with 60 additions and 10 deletions.
5 changes: 2 additions & 3 deletions doc/api/http.md
Original file line number Diff line number Diff line change
@@ -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
16 changes: 12 additions & 4 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
@@ -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;
23 changes: 23 additions & 0 deletions test/parallel/test-http-client-aborted.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}));
21 changes: 21 additions & 0 deletions test/parallel/test-http-client-no-error-after-aborted.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}));
}));
3 changes: 1 addition & 2 deletions test/parallel/test-http-client-timeout-on-connect.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}));
2 changes: 1 addition & 1 deletion test/parallel/test-http-writable-true-after-close.js
Original file line number Diff line number Diff line change
@@ -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();
}));

0 comments on commit 461bf36

Please sign in to comment.