From 2f1319967c740a0267fc84c7dfe31a09a2fd294d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 9 Jun 2020 17:08:53 +0200 Subject: [PATCH] http: make response.setTimeout() work Fixes: https://github.com/nodejs/node/issues/33734 PR-URL: https://github.com/nodejs/node/pull/34913 Reviewed-By: Luigi Pinca Reviewed-By: Ricky Zhou <0x19951125@gmail.com> Reviewed-By: Trivikram Kamat --- lib/_http_client.js | 20 ++++++++++++++----- .../test-http-client-response-timeout.js | 14 +++++++++++++ ...st-http-client-timeout-option-listeners.js | 4 ++-- 3 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-http-client-response-timeout.js diff --git a/lib/_http_client.js b/lib/_http_client.js index a54cfb2d1b2146..ef103314800ce3 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -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); @@ -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 @@ -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') @@ -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; diff --git a/test/parallel/test-http-client-response-timeout.js b/test/parallel/test-http-client-response-timeout.js new file mode 100644 index 00000000000000..7e44d83a831143 --- /dev/null +++ b/test/parallel/test-http-client-response-timeout.js @@ -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(); + })); +})); diff --git a/test/parallel/test-http-client-timeout-option-listeners.js b/test/parallel/test-http-client-timeout-option-listeners.js index dac89b5fd1a2bb..1122eaf79e46f8 100644 --- a/test/parallel/test-http-client-timeout-option-listeners.js +++ b/test/parallel/test-http-client-timeout-option-listeners.js @@ -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(); }));