http: make response.setTimeout() work

bnoordhuis · joesepi · commit d54a696a0fac · 2020-10-22T18:40:47.000-04:00
Fixes: nodejs#33734 PR-URL: nodejs#34913 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -534,8 +534,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);
@@ -645,6 +645,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
@@ -698,15 +699,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')
@@ -725,6 +727,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
@@ -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
@@ -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();
     }));
