add ClientRequest.prototype.clearTimeout (#19612)

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

src/js/node/_http_client.ts

@@ -4,6 +4,7 @@ const { checkIsHttpToken, validateFunction, validateInteger, validateBoolean } =
 const { urlToHttpOptions } = require("internal/url");
 const { isValidTLSArray } = require("internal/tls");
 const { validateHeaderName } = require("node:_http_common");
+const { getTimerDuration } = require("internal/timers");
 const {
   kBodyChunks,
   abortedSymbol,
@@ -40,7 +41,6 @@ const {
   reqSymbol,
   callCloseCallback,
   emitCloseNTAndComplete,
-  validateMsecs,
   ConnResetException,
 } = require("internal/http");
 
@@ -799,8 +799,9 @@ function ClientRequest(input, options, cb) {
   this[kHost] = host;
   this[kProtocol] = protocol;
 
-  const timeout = options.timeout;
-  if (timeout !== undefined && timeout !== 0) {
+  if (options.timeout !== undefined) {
+    const timeout = getTimerDuration(options.timeout, "timeout");
+    this.timeout = timeout;
     this.setTimeout(timeout, undefined);
   }
 
@@ -918,17 +919,18 @@ function ClientRequest(input, options, cb) {
       this.removeAllListeners("timeout");
     }
   };
+}
 
-  const onTimeout = () => {
-    this[kTimeoutTimer] = undefined;
-    this[kAbortController]?.abort();
-    this.emit("timeout");
-  };
+const ClientRequestPrototype = {
+  constructor: ClientRequest,
+  __proto__: OutgoingMessage.prototype,
 
-  this.setTimeout = (msecs, callback) => {
-    if (this.destroyed) return this;
+  setTimeout(msecs, callback) {
+    if (this.destroyed) {
+      return this;
+    }
 
-    this.timeout = msecs = validateMsecs(msecs, "timeout");
+    this.timeout = msecs = getTimerDuration(msecs, "msecs");
 
     // Attempt to clear an existing timer in both cases -
     //  even if it will be rescheduled we don't want to leak an existing timer.
@@ -942,7 +944,11 @@ function ClientRequest(input, options, cb) {
 
       this[kTimeoutTimer] = undefined;
     } else {
-      this[kTimeoutTimer] = setTimeout(onTimeout, msecs).unref();
+      this[kTimeoutTimer] = setTimeout(() => {
+        this[kTimeoutTimer] = undefined;
+        this[kAbortController]?.abort();
+        this.emit("timeout");
+      }, msecs).unref();
 
       if (callback !== undefined) {
         validateFunction(callback, "callback");
@@ -951,12 +957,11 @@ function ClientRequest(input, options, cb) {
     }
 
     return this;
-  };
-}
+  },
 
-const ClientRequestPrototype = {
-  constructor: ClientRequest,
-  __proto__: OutgoingMessage.prototype,
+  clearTimeout(cb) {
+    this.setTimeout(0, cb);
+  },
 
   get path() {
     return this[kPath];

src/js/node/_http_outgoing.ts

@@ -1,5 +1,5 @@
 const { Stream } = require("internal/stream");
-const { validateFunction, isUint8Array, validateString } = require("internal/validators");
+const { isUint8Array, validateString } = require("internal/validators");
 const { deprecate } = require("node:util");
 const ObjectDefineProperty = Object.defineProperty;
 const ObjectKeys = Object.keys;
@@ -13,8 +13,6 @@ const {
   kEmitState,
   ClientRequestEmitState,
   kEmptyObject,
-  validateMsecs,
-  timeoutTimerSymbol,
   kHandle,
   getHeader,
   setHeader,
@@ -367,28 +365,16 @@ const OutgoingMessagePrototype = {
   },
 
   setTimeout(msecs, callback) {
-    if (this.destroyed) return this;
+    if (this.callback) {
+      this.emit("timeout", callback);
+    }
 
-    this.timeout = msecs = validateMsecs(msecs, "timeout");
-
-    // Attempt to clear an existing timer in both cases -
-    //  even if it will be rescheduled we don't want to leak an existing timer.
-    clearTimeout(this[timeoutTimerSymbol]);
-
-    if (msecs === 0) {
-      if (callback != null) {
-        if (!$isCallable(callback)) validateFunction(callback, "callback");
-        this.removeListener("timeout", callback);
-      }
-
-      this[timeoutTimerSymbol] = undefined;
+    if (!this[fakeSocketSymbol]) {
+      this.once("socket", function socketSetTimeoutOnConnect(socket) {
+        socket.setTimeout(msecs, callback);
+      });
     } else {
-      this[timeoutTimerSymbol] = setTimeout(onTimeout.bind(this), msecs).unref();
-
-      if (callback != null) {
-        if (!$isCallable(callback)) validateFunction(callback, "callback");
-        this.once("timeout", callback);
-      }
+      this.socket.setTimeout(msecs);
     }
 
     return this;
@@ -592,17 +578,6 @@ ObjectDefineProperty(OutgoingMessage.prototype, "_headers", {
 });
 $setPrototypeDirect.$call(OutgoingMessage, Stream);
 
-function onTimeout() {
-  this[timeoutTimerSymbol] = undefined;
-  this[kAbortController]?.abort();
-  const handle = this[kHandle];
-
-  this.emit("timeout");
-  if (handle) {
-    handle.abort();
-  }
-}
-
 export default {
   OutgoingMessage,
   FakeSocket,

test/js/node/test/parallel/test-http-outgoing-settimeout.js

@@ -0,0 +1,30 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+const { OutgoingMessage } = require('http');
+
+{
+  // Tests for settimeout method with socket
+  const expectedMsecs = 42;
+  const outgoingMessage = new OutgoingMessage();
+  outgoingMessage.socket = {
+    setTimeout: common.mustCall((msecs) => {
+      assert.strictEqual(msecs, expectedMsecs);
+    })
+  };
+  outgoingMessage.setTimeout(expectedMsecs);
+}
+
+{
+  // Tests for settimeout method without socket
+  const expectedMsecs = 23;
+  const outgoingMessage = new OutgoingMessage();
+  outgoingMessage.setTimeout(expectedMsecs);
+
+  outgoingMessage.emit('socket', {
+    setTimeout: common.mustCall((msecs) => {
+      assert.strictEqual(msecs, expectedMsecs);
+    })
+  });
+}

test/js/node/test/parallel/test-http-timeout-client-warning.js

@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const http = require('http');
+const assert = require('assert');
+
+// Checks that the setTimeout duration overflow warning is emitted
+// synchronously and therefore contains a meaningful stacktrace.
+
+process.on('warning', common.mustCall((warning) => {
+  assert(warning.stack.includes(__filename));
+}));
+
+const server = http.createServer((req, resp) => resp.end());
+server.listen(common.mustCall(() => {
+  http.request(`http://localhost:${server.address().port}`)
+    .setTimeout(2 ** 40)
+    .on('response', common.mustCall(() => {
+      server.close();
+    }))
+    .end();
+}));

test/js/node/test/parallel/test-http-timeout-overflow.js

@@ -0,0 +1,51 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+'use strict';
+const common = require('../common');
+const http = require('http');
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end('OK');
+}));
+
+server.listen(0, function() {
+  function callback() {}
+
+  const req = http.request({
+    port: this.address().port,
+    path: '/',
+    agent: false
+  }, function(res) {
+    req.clearTimeout(callback);
+
+    res.on('end', common.mustCall(function() {
+      server.close();
+    }));
+
+    res.resume();
+  });
+
+  // Overflow signed int32
+  req.setTimeout(0xffffffff, callback);
+  req.end();
+});

test/js/node/test/parallel/test-http-timeout.js

@@ -0,0 +1,62 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+'use strict';
+const common = require('../common');
+
+const http = require('http');
+const Countdown = require('../common/countdown');
+const MAX_COUNT = 11;
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end('OK');
+}, MAX_COUNT));
+
+const agent = new http.Agent({ maxSockets: 1 });
+const countdown = new Countdown(MAX_COUNT, () => server.close());
+
+server.listen(0, function() {
+
+  for (let i = 0; i < MAX_COUNT; ++i) {
+    createRequest().end();
+  }
+
+  function callback() {}
+
+  function createRequest() {
+    const req = http.request(
+      { port: server.address().port, path: '/', agent: agent },
+      function(res) {
+        req.clearTimeout(callback);
+
+        res.on('end', function() {
+          countdown.dec();
+        });
+
+        res.resume();
+      }
+    );
+
+    req.setTimeout(1000, callback);
+    return req;
+  }
+});