-
-
Notifications
You must be signed in to change notification settings - Fork 714
Description
Bug Description
When the system time is changed forward by more than 5 minutes during a fetch() call, the operation terminates with UND_ERR_HEADERS_TIMEOUT.
As far as I can tell, this is due to how the timeout mechanism is implemented in lib/util/timers.js.
Timers above 1000 ms are realized with a custom, low resolution timer implementation, which relies on Date.now(). Therefore the timers are influenced by the system time. (I guess this would also mean the timeout would fire late/never when time is set back during an active fetch() operation too.)
In my testing, JS inbuilt setTimeout() is not impacted by system time changes.
Is this expected behavior?
Are there valid reasons why the timeout should be relative to the wall clock?
If this is a bug, could monotonic time functions like performance.now() or process.hrtime be used here instead?
Reproducible By
- Perform a long fetch operation
- Set system time forward by more than 5 minutes or
headersTimeout - Observe timeout error
Here is an unit test that verifies the early timeout for test/util.js:
test("do not timeout early on forward system time change", async (ctx) => {
let t = tspl(ctx, { plan: 3 });
let dateMock = ctx.mock.method(Date, "now");
dateMock.mock.mockImplementation(() =>
new Date("2024-08-22T10:00:00.000Z").valueOf()
);
const start = process.hrtime.bigint();
timers.setTimeout(() => {
const delta = getDelta(start, 2000);
t.ok(delta >= -1n, "timer fired early after system time change");
t.ok(
delta < ACCEPTABLE_DELTA,
"timer fired late after system time change"
);
}, 2000);
// Simulate user setting time forwards with active timer
setTimeout(
() =>
dateMock.mock.mockImplementation(() =>
new Date("2024-08-22T11:00:00.000Z").valueOf()
),
750
);
setTimeout(() => t.ok(true), 2500);
await t.completed;
});Expected Behavior
A fetch() calls timeout should not be influenced by system time.
Environment
Observed in Node v18.14.2, unit test written for undici on current main (69cfd97).