Skip to content

Commit

Permalink
http: make response.setTimeout() work
Browse files Browse the repository at this point in the history
Fixes: #33734

PR-URL: #34913
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
bnoordhuis authored and BethGriggs committed Oct 13, 2020
1 parent c995242 commit 2f13199
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 7 deletions.
20 changes: 15 additions & 5 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,8 @@ function socketOnData(d) {
socket.removeListener('end', socketOnEnd);
socket.removeListener('drain', ondrain);

if (req.timeoutCb)
socket.removeListener('timeout', req.timeoutCb);
if (req.timeoutCb) socket.removeListener('timeout', req.timeoutCb);
socket.removeListener('timeout', responseOnTimeout);

parser.finish();
freeParser(parser, req, socket);
Expand Down Expand Up @@ -634,6 +634,7 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
// Add our listener first, so that we guarantee socket cleanup
res.on('end', responseOnEnd);
req.on('prefinish', requestOnPrefinish);
socket.on('timeout', responseOnTimeout);

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

function responseOnEnd() {
const req = this.req;
const socket = req.socket;

if (req.socket && req.timeoutCb) {
req.socket.removeListener('timeout', emitRequestTimeout);
if (socket) {
if (req.timeoutCb) socket.removeListener('timeout', emitRequestTimeout);
socket.removeListener('timeout', responseOnTimeout);
}

req._ended = true;

if (!req.shouldKeepAlive) {
const socket = req.socket;
if (socket.writable) {
debug('AGENT socket.destroySoon()');
if (typeof socket.destroySoon === 'function')
Expand All @@ -714,6 +716,14 @@ function responseOnEnd() {
}
}

function responseOnTimeout() {
const req = this._httpMessage;
if (!req) return;
const res = req.res;
if (!res) return;
res.emit('timeout');
}

function requestOnPrefinish() {
const req = this;

Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-http-client-response-timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';
const common = require('../common');
const http = require('http');

const server = http.createServer((req, res) => res.flushHeaders());

server.listen(common.mustCall(() => {
const req =
http.get({ port: server.address().port }, common.mustCall((res) => {
res.on('timeout', common.mustCall(() => req.destroy()));
res.setTimeout(1);
server.close();
}));
}));
4 changes: 2 additions & 2 deletions test/parallel/test-http-client-timeout-option-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ const options = {
server.listen(0, options.host, common.mustCall(() => {
options.port = server.address().port;
doRequest(common.mustCall((numListeners) => {
assert.strictEqual(numListeners, 2);
assert.strictEqual(numListeners, 3);
doRequest(common.mustCall((numListeners) => {
assert.strictEqual(numListeners, 2);
assert.strictEqual(numListeners, 3);
server.close();
agent.destroy();
}));
Expand Down

0 comments on commit 2f13199

Please sign in to comment.