Skip to content

perf_hooks: fix timerify() inconsistency for rejected thenables - #65120

Open
batuhan-bas wants to merge 1 commit into
nodejs:mainfrom
batuhan-bas:perf-hooks-timerify-async-throw
Open

perf_hooks: fix timerify() inconsistency for rejected thenables#65120
batuhan-bas wants to merge 1 commit into
nodejs:mainfrom
batuhan-bas:perf-hooks-timerify-async-throw

Conversation

@batuhan-bas

Copy link
Copy Markdown

Summary

timerify() behaves inconsistently depending on whether the wrapped
function throws synchronously or returns a rejected thenable:

function f1() { throw new Error() }
async function f2() { throw new Error() }

const h1 = perf_hooks.createHistogram();
const h2 = perf_hooks.createHistogram();
const g1 = perf_hooks.performance.timerify(f1, { histogram: h1 });
const g2 = perf_hooks.performance.timerify(f2, { histogram: h2 });

try { g1(); } catch {}
await g2().catch(() => {});

h1.count === h2.count // expected true, actual false

g1() throws before the code ever reaches the recording step, so
nothing is recorded. g2() returns a rejected promise, and
timerify() records via Promise.prototype.finally(), which runs on
both fulfillment and rejection — so the async throw is recorded.

Fix

Use .then() with only an onFulfilled handler instead of
.finally(). This records only on fulfillment, matching the
synchronous case, while the rejection still propagates through the
returned promise unchanged. It also avoids assuming the returned
thenable implements finally(), which isn't part of the minimal
thenable contract (only then() is required).

Test plan

  • Added test/parallel/test-perf-hooks-timerify-async-error.js,
    asserting a rejected thenable produces no histogram record and no
    'function' performance entry.
  • Ran the full existing test-perf-hooks-timerify-* suite locally,
    all passing.

Fixes: #42743

timerify() used Promise.prototype.finally() to record a function's
duration and histogram entry once its returned thenable settled,
regardless of whether it fulfilled or rejected. This meant a
timerified async function that throws would still be recorded, while
a timerified sync function that throws never reaches the recording
step at all, since it throws before returning.

Switch to then() with only an onFulfilled handler so a rejected
thenable behaves like a synchronous throw: no performance entry and
no histogram record, and the rejection still propagates unchanged.

Fixes: nodejs#42743
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

performance.timerify(fn) behave inconsistently for sync/async functions

2 participants