Skip to content

Commit 461bf36

Browse files
ronagTrott
authored andcommitted
http: fix test where aborted should not be emitted
PR-URL: #20077 Fixes: #20107 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 2550ddb commit 461bf36

6 files changed

+60
-10
lines changed

doc/api/http.md

+2-3
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,8 @@ added: v0.3.8
541541
-->
542542

543543
Marks the request as aborting. Calling this will cause remaining data
544-
in the response to be dropped and the socket to be destroyed.
544+
in the response to be dropped and the socket to be destroyed. After
545+
calling this method no further errors will be emitted.
545546

546547
### request.aborted
547548
<!-- YAML
@@ -2142,8 +2143,6 @@ will be emitted in the following order:
21422143
* `'socket'`
21432144
* (`req.abort()` called here)
21442145
* `'abort'`
2145-
* `'error'` with an error with message `'Error: socket hang up'` and code
2146-
`'ECONNRESET'`
21472146
* `'close'`
21482147

21492148
If `req.abort()` is called after the response is received, the following events

lib/_http_client.js

+12-4
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,9 @@ function socketCloseListener() {
374374
// receive a response. The error needs to
375375
// fire on the request.
376376
req.socket._hadError = true;
377-
req.emit('error', connResetException('socket hang up'));
377+
if (!req.aborted) {
378+
req.emit('error', connResetException('socket hang up'));
379+
}
378380
}
379381
req.emit('close');
380382
}
@@ -400,7 +402,9 @@ function socketErrorListener(err) {
400402
// For Safety. Some additional errors might fire later on
401403
// and we need to make sure we don't double-fire the error event.
402404
req.socket._hadError = true;
403-
req.emit('error', err);
405+
if (!req.aborted) {
406+
req.emit('error', err);
407+
}
404408
}
405409

406410
// Handle any pending data
@@ -434,7 +438,9 @@ function socketOnEnd() {
434438
// If we don't have a response then we know that the socket
435439
// ended prematurely and we need to emit an error on the request.
436440
req.socket._hadError = true;
437-
req.emit('error', connResetException('socket hang up'));
441+
if (!req.aborted) {
442+
req.emit('error', connResetException('socket hang up'));
443+
}
438444
}
439445
if (parser) {
440446
parser.finish();
@@ -457,7 +463,9 @@ function socketOnData(d) {
457463
freeParser(parser, req, socket);
458464
socket.destroy();
459465
req.socket._hadError = true;
460-
req.emit('error', ret);
466+
if (!req.aborted) {
467+
req.emit('error', ret);
468+
}
461469
} else if (parser.incoming && parser.incoming.upgrade) {
462470
// Upgrade (if status code 101) or CONNECT
463471
var bytesParsed = ret;
+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const http = require('http');
5+
const assert = require('assert');
6+
7+
const server = http.createServer(common.mustCall(function(req, res) {
8+
req.on('aborted', common.mustCall(function() {
9+
assert.strictEqual(this.aborted, true);
10+
server.close();
11+
}));
12+
assert.strictEqual(req.aborted, false);
13+
res.write('hello');
14+
}));
15+
16+
server.listen(0, common.mustCall(() => {
17+
const req = http.get({
18+
port: server.address().port,
19+
headers: { connection: 'keep-alive' }
20+
}, common.mustCall((res) => {
21+
req.abort();
22+
}));
23+
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const http = require('http');
5+
6+
const server = http.createServer(common.mustCall((req, res) => {
7+
res.write('hello');
8+
}));
9+
10+
server.listen(0, common.mustCall(() => {
11+
const req = http.get({
12+
port: server.address().port
13+
}, common.mustCall((res) => {
14+
req.on('error', common.mustNotCall());
15+
req.abort();
16+
req.socket.destroy(new Error());
17+
req.on('close', common.mustCall(() => {
18+
server.close();
19+
}));
20+
}));
21+
}));

test/parallel/test-http-client-timeout-on-connect.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ server.listen(0, common.localhostIPv4, common.mustCall(() => {
2323
}));
2424
}));
2525
req.on('timeout', common.mustCall(() => req.abort()));
26-
req.on('error', common.mustCall((err) => {
27-
assert.strictEqual(err.message, 'socket hang up');
26+
req.on('abort', common.mustCall(() => {
2827
server.close();
2928
}));
3029
}));

test/parallel/test-http-writable-true-after-close.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const server = createServer(common.mustCall((req, res) => {
3434
}));
3535
}).listen(0, () => {
3636
external = get(`http://127.0.0.1:${server.address().port}`);
37-
external.on('error', common.mustCall(() => {
37+
external.on('abort', common.mustCall(() => {
3838
server.close();
3939
internal.close();
4040
}));

0 commit comments

Comments
 (0)