Skip to content

Commit 2f13199

Browse files
bnoordhuisBethGriggs
authored andcommitted
http: make response.setTimeout() work
Fixes: #33734 PR-URL: #34913 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ricky Zhou <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
1 parent c995242 commit 2f13199

3 files changed

+31
-7
lines changed

lib/_http_client.js

+15-5
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,8 @@ function socketOnData(d) {
525525
socket.removeListener('end', socketOnEnd);
526526
socket.removeListener('drain', ondrain);
527527

528-
if (req.timeoutCb)
529-
socket.removeListener('timeout', req.timeoutCb);
528+
if (req.timeoutCb) socket.removeListener('timeout', req.timeoutCb);
529+
socket.removeListener('timeout', responseOnTimeout);
530530

531531
parser.finish();
532532
freeParser(parser, req, socket);
@@ -634,6 +634,7 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
634634
// Add our listener first, so that we guarantee socket cleanup
635635
res.on('end', responseOnEnd);
636636
req.on('prefinish', requestOnPrefinish);
637+
socket.on('timeout', responseOnTimeout);
637638

638639
// If the user did not listen for the 'response' event, then they
639640
// can't possibly read the data, so we ._dump() it into the void
@@ -687,15 +688,16 @@ function responseKeepAlive(req) {
687688

688689
function responseOnEnd() {
689690
const req = this.req;
691+
const socket = req.socket;
690692

691-
if (req.socket && req.timeoutCb) {
692-
req.socket.removeListener('timeout', emitRequestTimeout);
693+
if (socket) {
694+
if (req.timeoutCb) socket.removeListener('timeout', emitRequestTimeout);
695+
socket.removeListener('timeout', responseOnTimeout);
693696
}
694697

695698
req._ended = true;
696699

697700
if (!req.shouldKeepAlive) {
698-
const socket = req.socket;
699701
if (socket.writable) {
700702
debug('AGENT socket.destroySoon()');
701703
if (typeof socket.destroySoon === 'function')
@@ -714,6 +716,14 @@ function responseOnEnd() {
714716
}
715717
}
716718

719+
function responseOnTimeout() {
720+
const req = this._httpMessage;
721+
if (!req) return;
722+
const res = req.res;
723+
if (!res) return;
724+
res.emit('timeout');
725+
}
726+
717727
function requestOnPrefinish() {
718728
const req = this;
719729

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
const common = require('../common');
3+
const http = require('http');
4+
5+
const server = http.createServer((req, res) => res.flushHeaders());
6+
7+
server.listen(common.mustCall(() => {
8+
const req =
9+
http.get({ port: server.address().port }, common.mustCall((res) => {
10+
res.on('timeout', common.mustCall(() => req.destroy()));
11+
res.setTimeout(1);
12+
server.close();
13+
}));
14+
}));

test/parallel/test-http-client-timeout-option-listeners.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ const options = {
2424
server.listen(0, options.host, common.mustCall(() => {
2525
options.port = server.address().port;
2626
doRequest(common.mustCall((numListeners) => {
27-
assert.strictEqual(numListeners, 2);
27+
assert.strictEqual(numListeners, 3);
2828
doRequest(common.mustCall((numListeners) => {
29-
assert.strictEqual(numListeners, 2);
29+
assert.strictEqual(numListeners, 3);
3030
server.close();
3131
agent.destroy();
3232
}));

0 commit comments

Comments
 (0)