Fixes #3588 (#3590)

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

2 files changed, 149 insertions, 63 deletions

diff --git a/src/js/node/http.ts b/src/js/node/http.ts
index 15b060d5f..a745f9b32 100644
--- a/src/js/node/http.ts
+++ b/src/js/node/http.ts
@@ -109,6 +109,28 @@ function isValidTLSArray(obj) {
   }
 }
 
+class ERR_INVALID_ARG_TYPE extends TypeError {
+  constructor(name, expected, actual) {
+    super(`The ${name} argument must be of type ${expected}. Received type ${typeof actual}`);
+    this.code = "ERR_INVALID_ARG_TYPE";
+  }
+}
+
+function validateMsecs(numberlike: any, field: string) {
+  if (typeof numberlike !== "number" || numberlike < 0) {
+    throw new ERR_INVALID_ARG_TYPE(field, "number", numberlike);
+  }
+
+  return numberlike;
+}
+function validateFunction(callable: any, field: string) {
+  if (typeof callable !== "function") {
+    throw new ERR_INVALID_ARG_TYPE(field, "Function", callable);
+  }
+
+  return callable;
+}
+
 function getHeader(headers, name) {
   if (!headers) return;
   const result = headers.get(name);
@@ -792,12 +814,13 @@ export class OutgoingMessage extends Writable {
   headersSent = false;
   sendDate = true;
   req;
+  timeout;
 
   #finished = false;
   [kEndCalled] = false;
 
   #fakeSocket;
-  #timeoutTimer: Timer | null = null;
+  #timeoutTimer?: Timer;
   [kAbortController]: AbortController | null = null;
 
   // Express "compress" package uses this
@@ -894,21 +917,41 @@ export class OutgoingMessage extends Writable {
   [kClearTimeout]() {
     if (this.#timeoutTimer) {
       clearTimeout(this.#timeoutTimer);
-      this.#timeoutTimer = null;
+      this.removeAllListeners("timeout");
+      this.#timeoutTimer = undefined;
     }
   }
 
+  #onTimeout() {
+    this.#timeoutTimer = undefined;
+    this[kAbortController]?.abort();
+    this.emit("timeout");
+  }
+
   setTimeout(msecs, callback) {
-    if (this.#timeoutTimer) return this;
-    if (callback) {
-      this.on("timeout", callback);
-    }
+    if (this.destroyed) return this;
+
+    this.timeout = msecs = validateMsecs(msecs, "msecs");
 
-    this.#timeoutTimer = setTimeout(async () => {
-      this.#timeoutTimer = null;
-      this[kAbortController]?.abort();
-      this.emit("timeout");
-    }, 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.
+    clearTimeout(this.#timeoutTimer!);
+
+    if (msecs === 0) {
+      if (callback !== undefined) {
+        validateFunction(callback, "callback");
+        this.removeListener("timeout", callback);
+      }
+
+      this.#timeoutTimer = undefined;
+    } else {
+      this.#timeoutTimer = setTimeout(this.#onTimeout.bind(this), msecs).unref();
+
+      if (callback !== undefined) {
+        validateFunction(callback, "callback");
+        this.once("timeout", callback);
+      }
+    }
 
     return this;
   }
@@ -1155,7 +1198,7 @@ export class ClientRequest extends OutgoingMessage {
   #fetchRequest;
   #signal: AbortSignal | null = null;
   [kAbortController]: AbortController | null = null;
-  #timeoutTimer: Timer | null = null;
+  #timeoutTimer?: Timer = undefined;
   #options;
   #finished;
 
@@ -1224,6 +1267,9 @@ export class ClientRequest extends OutgoingMessage {
           redirect: "manual",
           verbose: Boolean(__DEBUG__),
           signal: this[kAbortController].signal,
+
+          // Timeouts are handled via this.setTimeout.
+          timeout: false,
         },
       )
         .then(response => {
@@ -1348,8 +1394,6 @@ export class ClientRequest extends OutgoingMessage {
 
     this.#socketPath = options.socketPath;
 
-    if (options.timeout !== undefined) this.setTimeout(options.timeout, null);
-
     const signal = options.signal;
     if (signal) {
       //We still want to control abort function and timeout so signal call our AbortController
@@ -1427,7 +1471,12 @@ export class ClientRequest extends OutgoingMessage {
     this.#reusedSocket = false;
     this.#host = host;
     this.#protocol = protocol;
-    this.#timeoutTimer = null;
+
+    var timeout = options.timeout;
+    if (timeout !== undefined && timeout !== 0) {
+      this.setTimeout(timeout, undefined);
+    }
+
     const headersArray = ArrayIsArray(headers);
     if (!headersArray) {
       var headers = options.headers;
@@ -1482,17 +1531,8 @@ export class ClientRequest extends OutgoingMessage {
 
     // this[kUniqueHeaders] = parseUniqueHeadersOption(options.uniqueHeaders);
 
-    var optsWithoutSignal = options;
-    if (optsWithoutSignal.signal) {
-      optsWithoutSignal = ObjectAssign({}, options);
-      delete optsWithoutSignal.signal;
-    }
+    var { signal: _signal, ...optsWithoutSignal } = options;
     this.#options = optsWithoutSignal;
-
-    var timeout = options.timeout;
-    if (timeout) {
-      this.setTimeout(timeout);
-    }
   }
 
   setSocketKeepAlive(enable = true, initialDelay = 0) {
@@ -1505,21 +1545,41 @@ export class ClientRequest extends OutgoingMessage {
   [kClearTimeout]() {
     if (this.#timeoutTimer) {
       clearTimeout(this.#timeoutTimer);
-      this.#timeoutTimer = null;
+      this.#timeoutTimer = undefined;
+      this.removeAllListeners("timeout");
     }
   }
 
-  setTimeout(msecs, callback?) {
-    if (this.#timeoutTimer) return this;
-    if (callback) {
-      this.on("timeout", callback);
-    }
+  #onTimeout() {
+    this.#timeoutTimer = undefined;
+    this[kAbortController]?.abort();
+    this.emit("timeout");
+  }
 
-    this.#timeoutTimer = setTimeout(async () => {
-      this.#timeoutTimer = null;
-      this[kAbortController]?.abort();
-      this.emit("timeout");
-    }, msecs);
+  setTimeout(msecs, callback) {
+    if (this.destroyed) return this;
+
+    this.timeout = msecs = validateMsecs(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.
+    clearTimeout(this.#timeoutTimer!);
+
+    if (msecs === 0) {
+      if (callback !== undefined) {
+        validateFunction(callback, "callback");
+        this.removeListener("timeout", callback);
+      }
+
+      this.#timeoutTimer = undefined;
+    } else {
+      this.#timeoutTimer = setTimeout(this.#onTimeout.bind(this), msecs).unref();
+
+      if (callback !== undefined) {
+        validateFunction(callback, "callback");
+        this.once("timeout", callback);
+      }
+    }
 
     return this;
   }
diff --git a/src/js/out/modules/node/http.js b/src/js/out/modules/node/http.js
index f07dcc2e0..13ee7fded 100644
--- a/src/js/out/modules/node/http.js
+++ b/src/js/out/modules/node/http.js
@@ -14,6 +14,14 @@ var checkInvalidHeaderChar = function(val) {
         return !1;
     return !0;
   }
+}, validateMsecs = function(numberlike, field) {
+  if (typeof numberlike !== "number" || numberlike < 0)
+    throw new ERR_INVALID_ARG_TYPE(field, "number", numberlike);
+  return numberlike;
+}, validateFunction = function(callable, field) {
+  if (typeof callable !== "function")
+    throw new ERR_INVALID_ARG_TYPE(field, "Function", callable);
+  return callable;
 }, getHeader = function(headers, name) {
   if (!headers)
     return;
@@ -108,7 +116,15 @@ var headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/, validateHeaderName = (name, lab
   if (checkInvalidHeaderChar(value))
     throw new Error("ERR_INVALID_CHAR");
 }, { URL } = globalThis, { newArrayWithSize, String, Object, Array } = globalThis[Symbol.for("Bun.lazy")]("primordials"), globalReportError = globalThis.reportError, setTimeout = globalThis.setTimeout, fetch = Bun.fetch, nop = () => {
-}, __DEBUG__ = process.env.__DEBUG__, debug = __DEBUG__ ? (...args) => console.log("node:http", ...args) : nop, kEmptyObject = Object.freeze(Object.create(null)), kOutHeaders = Symbol.for("kOutHeaders"), kEndCalled = Symbol.for("kEndCalled"), kAbortController = Symbol.for("kAbortController"), kClearTimeout = Symbol("kClearTimeout"), kCorked = Symbol.for("kCorked"), searchParamsSymbol = Symbol.for("query"), StringPrototypeSlice = String.prototype.slice, StringPrototypeStartsWith = String.prototype.startsWith, StringPrototypeToUpperCase = String.prototype.toUpperCase, StringPrototypeIncludes = String.prototype.includes, StringPrototypeCharCodeAt = String.prototype.charCodeAt, StringPrototypeIndexOf = String.prototype.indexOf, ArrayIsArray = Array.isArray, RegExpPrototypeExec = RegExp.prototype.exec, ObjectAssign = Object.assign, ObjectPrototypeHasOwnProperty = Object.prototype.hasOwnProperty, INVALID_PATH_REGEX = /[^\u0021-\u00ff]/, NODE_HTTP_WARNING = "WARN: Agent is mostly unused in Bun's implementation of http. If you see strange behavior, this is probably the cause.", _defaultHTTPSAgent, kInternalRequest = Symbol("kInternalRequest"), kInternalSocketData = Symbol.for("::bunternal::"), kEmptyBuffer = Buffer.alloc(0), FakeSocket = class Socket extends Duplex {
+}, __DEBUG__ = process.env.__DEBUG__, debug = __DEBUG__ ? (...args) => console.log("node:http", ...args) : nop, kEmptyObject = Object.freeze(Object.create(null)), kOutHeaders = Symbol.for("kOutHeaders"), kEndCalled = Symbol.for("kEndCalled"), kAbortController = Symbol.for("kAbortController"), kClearTimeout = Symbol("kClearTimeout"), kCorked = Symbol.for("kCorked"), searchParamsSymbol = Symbol.for("query"), StringPrototypeSlice = String.prototype.slice, StringPrototypeStartsWith = String.prototype.startsWith, StringPrototypeToUpperCase = String.prototype.toUpperCase, StringPrototypeIncludes = String.prototype.includes, StringPrototypeCharCodeAt = String.prototype.charCodeAt, StringPrototypeIndexOf = String.prototype.indexOf, ArrayIsArray = Array.isArray, RegExpPrototypeExec = RegExp.prototype.exec, ObjectAssign = Object.assign, ObjectPrototypeHasOwnProperty = Object.prototype.hasOwnProperty, INVALID_PATH_REGEX = /[^\u0021-\u00ff]/, NODE_HTTP_WARNING = "WARN: Agent is mostly unused in Bun's implementation of http. If you see strange behavior, this is probably the cause.", _defaultHTTPSAgent, kInternalRequest = Symbol("kInternalRequest"), kInternalSocketData = Symbol.for("::bunternal::"), kEmptyBuffer = Buffer.alloc(0);
+
+class ERR_INVALID_ARG_TYPE extends TypeError {
+  constructor(name, expected, actual) {
+    super(`The ${name} argument must be of type ${expected}. Received type ${typeof actual}`);
+    this.code = "ERR_INVALID_ARG_TYPE";
+  }
+}
+var FakeSocket = class Socket extends Duplex {
   bytesRead = 0;
   bytesWritten = 0;
   connecting = !1;
@@ -514,10 +530,11 @@ class OutgoingMessage extends Writable {
   headersSent = !1;
   sendDate = !0;
   req;
+  timeout;
   #finished = !1;
   [kEndCalled] = !1;
   #fakeSocket;
-  #timeoutTimer = null;
+  #timeoutTimer;
   [kAbortController] = null;
   _implicitHeader() {
   }
@@ -592,16 +609,21 @@ class OutgoingMessage extends Writable {
   }
   [kClearTimeout]() {
     if (this.#timeoutTimer)
-      clearTimeout(this.#timeoutTimer), this.#timeoutTimer = null;
+      clearTimeout(this.#timeoutTimer), this.removeAllListeners("timeout"), this.#timeoutTimer = void 0;
+  }
+  #onTimeout() {
+    this.#timeoutTimer = void 0, this[kAbortController]?.abort(), this.emit("timeout");
   }
   setTimeout(msecs, callback) {
-    if (this.#timeoutTimer)
+    if (this.destroyed)
       return this;
-    if (callback)
-      this.on("timeout", callback);
-    return this.#timeoutTimer = setTimeout(async () => {
-      this.#timeoutTimer = null, this[kAbortController]?.abort(), this.emit("timeout");
-    }, msecs), this;
+    if (this.timeout = msecs = validateMsecs(msecs, "msecs"), clearTimeout(this.#timeoutTimer), msecs === 0) {
+      if (callback !== void 0)
+        validateFunction(callback, "callback"), this.removeListener("timeout", callback);
+      this.#timeoutTimer = void 0;
+    } else if (this.#timeoutTimer = setTimeout(this.#onTimeout.bind(this), msecs).unref(), callback !== void 0)
+      validateFunction(callback, "callback"), this.once("timeout", callback);
+    return this;
   }
 }
 
@@ -780,7 +802,7 @@ class ClientRequest extends OutgoingMessage {
   #fetchRequest;
   #signal = null;
   [kAbortController] = null;
-  #timeoutTimer = null;
+  #timeoutTimer = void 0;
   #options;
   #finished;
   get path() {
@@ -827,7 +849,8 @@ class ClientRequest extends OutgoingMessage {
         body: body && method !== "GET" && method !== "HEAD" && method !== "OPTIONS" ? body : void 0,
         redirect: "manual",
         verbose: Boolean(__DEBUG__),
-        signal: this[kAbortController].signal
+        signal: this[kAbortController].signal,
+        timeout: !1
       }).then((response) => {
         var res = this.#res = new IncomingMessage(response, {
           type: "response",
@@ -910,8 +933,7 @@ class ClientRequest extends OutgoingMessage {
     const defaultPort = protocol === "https:" ? 443 : 80;
     this.#port = options.port || options.defaultPort || this.#agent?.defaultPort || defaultPort, this.#useDefaultPort = this.#port === defaultPort;
     const host = this.#host = options.host = validateHost(options.hostname, "hostname") || validateHost(options.host, "host") || "localhost";
-    if (this.#socketPath = options.socketPath, options.timeout !== void 0)
-      this.setTimeout(options.timeout, null);
+    this.#socketPath = options.socketPath;
     const signal = options.signal;
     if (signal)
       signal.addEventListener("abort", () => {
@@ -932,7 +954,11 @@ class ClientRequest extends OutgoingMessage {
     var _joinDuplicateHeaders = options.joinDuplicateHeaders;
     if (this.#joinDuplicateHeaders = _joinDuplicateHeaders, this.#path = options.path || "/", cb)
       this.once("response", cb);
-    if (__DEBUG__ && debug(`new ClientRequest: ${this.#method} ${this.#protocol}//${this.#host}:${this.#port}${this.#path}`), this.#finished = !1, this.#res = null, this.#upgradeOrConnect = !1, this.#parser = null, this.#maxHeadersCount = null, this.#reusedSocket = !1, this.#host = host, this.#protocol = protocol, this.#timeoutTimer = null, !ArrayIsArray(headers)) {
+    __DEBUG__ && debug(`new ClientRequest: ${this.#method} ${this.#protocol}//${this.#host}:${this.#port}${this.#path}`), this.#finished = !1, this.#res = null, this.#upgradeOrConnect = !1, this.#parser = null, this.#maxHeadersCount = null, this.#reusedSocket = !1, this.#host = host, this.#protocol = protocol;
+    var timeout = options.timeout;
+    if (timeout !== void 0 && timeout !== 0)
+      this.setTimeout(timeout, void 0);
+    if (!ArrayIsArray(headers)) {
       var headers = options.headers;
       if (headers)
         for (let key in headers)
@@ -941,13 +967,8 @@ class ClientRequest extends OutgoingMessage {
       if (auth && !this.getHeader("Authorization"))
         this.setHeader("Authorization", "Basic " + Buffer.from(auth).toString("base64"));
     }
-    var optsWithoutSignal = options;
-    if (optsWithoutSignal.signal)
-      optsWithoutSignal = ObjectAssign({}, options), delete optsWithoutSignal.signal;
+    var { signal: _signal, ...optsWithoutSignal } = options;
     this.#options = optsWithoutSignal;
-    var timeout = options.timeout;
-    if (timeout)
-      this.setTimeout(timeout);
   }
   setSocketKeepAlive(enable = !0, initialDelay = 0) {
     __DEBUG__ && debug(`${NODE_HTTP_WARNING}\n`, "WARN: ClientRequest.setSocketKeepAlive is a no-op");
@@ -957,16 +978,21 @@ class ClientRequest extends OutgoingMessage {
   }
   [kClearTimeout]() {
     if (this.#timeoutTimer)
-      clearTimeout(this.#timeoutTimer), this.#timeoutTimer = null;
+      clearTimeout(this.#timeoutTimer), this.#timeoutTimer = void 0, this.removeAllListeners("timeout");
+  }
+  #onTimeout() {
+    this.#timeoutTimer = void 0, this[kAbortController]?.abort(), this.emit("timeout");
   }
   setTimeout(msecs, callback) {
-    if (this.#timeoutTimer)
+    if (this.destroyed)
       return this;
-    if (callback)
-      this.on("timeout", callback);
-    return this.#timeoutTimer = setTimeout(async () => {
-      this.#timeoutTimer = null, this[kAbortController]?.abort(), this.emit("timeout");
-    }, msecs), this;
+    if (this.timeout = msecs = validateMsecs(msecs, "msecs"), clearTimeout(this.#timeoutTimer), msecs === 0) {
+      if (callback !== void 0)
+        validateFunction(callback, "callback"), this.removeListener("timeout", callback);
+      this.#timeoutTimer = void 0;
+    } else if (this.#timeoutTimer = setTimeout(this.#onTimeout.bind(this), msecs).unref(), callback !== void 0)
+      validateFunction(callback, "callback"), this.once("timeout", callback);
+    return this;
   }
 }
 var tokenRegExp = /^[\^_`a-zA-Z\-0-9!#$%&'*+.|~]+$/, METHODS = [