Fix timer guards to avoid TypeError under fake‐timers and polyfilled … (#4213)

1ly4s0 · Uzlopak · web-flow · commit 995f064157b1 · 2025-06-26T13:41:09.000-07:00
* Fix timer guards to avoid TypeError under fake‐timers and polyfilled environments

* Remove styling changes

* Add tests

* Add comments

* fix(lint): satisfy eslint stylistic rules for EOF, indent and jest globals

* Update lib/dispatcher/client-h1.js

* fix linting issue

* use setTimeout from node:timers to increase chance of having node specific helper methods for timers,  if refresh is not existing,  create a new timeout for refreshTimeout

* Apply suggestions from code review

---------

Co-authored-by: Aras Abbasi &lt;aras.abbasi@googlemail.com&gt;
diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js
@@ -249,7 +249,7 @@ class Parser {
           this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this))
         } else {
           this.timeout = setTimeout(onParserTimeout, delay, new WeakRef(this))
-          this.timeout.unref()
+          this.timeout?.unref()
         }
       }
 
diff --git a/lib/util/timers.js b/lib/util/timers.js
@@ -188,19 +188,21 @@ function onTick () {
 }
 
 function refreshTimeout () {
-  // If the fastNowTimeout is already set, refresh it.
-  if (fastNowTimeout) {
+  // If the fastNowTimeout is already set and the Timer has the refresh()-
+  // method available, call it to refresh the timer.
+  // Some timer objects returned by setTimeout may not have a .refresh()
+  // method (e.g. mocked timers in tests).
+  if (fastNowTimeout?.refresh) {
     fastNowTimeout.refresh()
-  // fastNowTimeout is not instantiated yet, create a new Timer.
+    // fastNowTimeout is not instantiated yet or refresh is not availabe,
+    // create a new Timer.
   } else {
     clearTimeout(fastNowTimeout)
     fastNowTimeout = setTimeout(onTick, TICK_MS)
-
-    // If the Timer has an unref method, call it to allow the process to exit if
-    // there are no other active handles.
-    if (fastNowTimeout.unref) {
-      fastNowTimeout.unref()
-    }
+    // If the Timer has an unref method, call it to allow the process to exit,
+    // if there are no other active handles. When using fake timers or mocked
+    // environments (like Jest), .unref() may not be defined,
+    fastNowTimeout?.unref()
   }
 }
 
diff --git a/test/jest/parser-timeout.test.js b/test/jest/parser-timeout.test.js
@@ -0,0 +1,41 @@
+'use strict'
+/* global jest, describe, it, beforeEach, afterEach, expect */
+
+// test/jest/parser-timeout.test.js
+const EventEmitter = require('events')
+const connectH1 = require('../../lib/dispatcher/client-h1')
+const { kParser } = require('../../lib/core/symbols')
+
+// DummySocket extends EventEmitter to support .on/.off/.read()
+class DummySocket extends EventEmitter {
+  constructor () {
+    super()
+    this.destroyed = false
+    this.errored = null
+  }
+
+  read () {
+    return null
+  }
+}
+
+const dummyClient = {
+  [Symbol.for('kMaxHeadersSize')]: 1024,
+  [Symbol.for('kMaxResponseSize')]: 1024,
+  [Symbol.for('kQueue')]: [],
+  [Symbol.for('kRunningIdx')]: 0,
+  headersTimeout: 100,
+  bodyTimeout: 100
+}
+
+describe('Parser#setTimeout under fake timers', () => {
+  beforeEach(() => jest.useFakeTimers('modern'))
+  afterEach(() => jest.useRealTimers())
+
+  it('does not throw when calling setTimeout under fake timers', async () => {
+    const socket = new DummySocket()
+    await connectH1(dummyClient, socket) // connectH1 creates Parser -> socket[kParser]
+    const parser = socket[kParser]
+    expect(() => parser.setTimeout(200, 0)).not.toThrow()
+  })
+})
diff --git a/test/jest/util-timers.test.js b/test/jest/util-timers.test.js
@@ -0,0 +1,26 @@
+'use strict'
+/* global jest, describe, it, beforeEach, afterEach, expect */
+
+// test/jest/util-timers.test.js
+const timers = require('../../lib/util/timers')
+
+describe('util/timers under fake timers', () => {
+  beforeEach(() => jest.useFakeTimers('modern'))
+  afterEach(() => {
+    jest.useRealTimers()
+    try {
+      timers.reset()
+    } catch {}
+  })
+
+  it('setFastTimeout + clearFastTimeout does not throw', () => {
+    const fast = timers.setFastTimeout(() => {}, 2000)
+    expect(typeof fast.refresh).toBe('function')
+    expect(() => timers.clearFastTimeout(fast)).not.toThrow()
+  })
+
+  it('short setTimeout + clearTimeout does not throw', () => {
+    const t = timers.setTimeout(() => {}, 10)
+    expect(() => timers.clearTimeout(t)).not.toThrow()
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -249,7 +249,7 @@ class Parser {`
`249`	`249`	`this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this))`
`250`	`250`	`} else {`
`251`	`251`	`this.timeout = setTimeout(onParserTimeout, delay, new WeakRef(this))`
`252`		`- this.timeout.unref()`
	`252`	`+ this.timeout?.unref()`
`253`	`253`	`}`
`254`	`254`	`}`
`255`	`255`