fix: clear timeout before invoking the callback - #2082
xianjianlf2 wants to merge 1 commit into
Conversation
lbesecker195
left a comment
There was a problem hiding this comment.
Reproduced this on master. The task calls back in time and the final callback throws, so clearTimeout never runs. The caller catches the error, but 20ms later the same callback is called again with ETIMEDOUT. That happened both when the task called back synchronously and when it called back from a setTimeout.
With this branch, each of those cases gets exactly one call and the thrown error still reaches the caller. A task that finishes in time without throwing still gets its result, and a slow task still gets ETIMEDOUT.
timer is assigned before fn(...args) runs, so moving clearTimeout ahead of the callback also covers a task that calls back synchronously. The new test fails on master (expected 2 to equal 1) and passes here. npm test passes: 691 tests, up from 690 on master, and lint makes no changes.
LGTM.
When a task completes before its timeout but the final callback throws,
async.timeoutnever reachesclearTimeout. Even if the caller catches that exception, the timer fires later and invokes the callback a second time withETIMEDOUT.Clear the timer before invoking the final callback. The callback's exception still propagates unchanged. Add a regression test that catches the original exception and verifies that no later timeout callback arrives; it records two calls before the fix and one afterward.
Validation:
npm testpasses (lint and 691 Node tests). FirefoxHeadless passes all 679 browser tests.AI assistance: implemented and tested with OpenAI Codex.