perf_hooks: do not record timerify entry for async throw

`timerify` used `result.finally(processComplete)` for thenables,
which recorded a histogram sample and enqueued a `function`
PerformanceEntry on async rejection. The sync-throw path never
reached `processComplete` because the throw happens inside
ReflectApply, so async- and sync-throwing variants of the same
function produced different observable outputs.

Use `result.then(onFulfilledTimerified)` with no onRejected so
rejections propagate unchanged and `processComplete` runs only on
fulfillment. Relax the thenable check from `finally` to `then`,
since a thenable is only required to implement `then`.

Fixes: #42743
Signed-off-by: Maruthan G <maruthang4@gmail.com>

Author: maruthang

lib/internal/perf/timerify.js

@@ -52,6 +52,11 @@ function processComplete(name, start, args, histogram) {
   enqueue(entry);
 }
 
+function onFulfilledTimerified(name, start, args, histogram, value) {
+  processComplete(name, start, args, histogram);
+  return value;
+}
+
 function timerify(fn, options = kEmptyObject) {
   validateFunction(fn, 'fn');
 
@@ -74,10 +79,18 @@ function timerify(fn, options = kEmptyObject) {
     const result = isConstructorCall ?
       ReflectConstruct(fn, args, fn) :
       ReflectApply(fn, this, args);
-    if (!isConstructorCall && typeof result?.finally === 'function') {
-      return result.finally(
+    if (!isConstructorCall && typeof result?.then === 'function') {
+      // Use `then` (not `finally`) so that a rejected thenable does NOT
+      // produce a performance entry or histogram record. This mirrors the
+      // behavior of synchronously-thrown errors, which never reach
+      // `processComplete`. Refs: https://github.com/nodejs/node/issues/42743
+      // Also, thenables are only required to implement `then`; relying on
+      // `finally` would break for minimal thenable implementations.
+      // Pass only an `onFulfilled` handler so any rejection propagates
+      // unchanged through the returned promise.
+      return result.then(
         FunctionPrototypeBind(
-          processComplete,
+          onFulfilledTimerified,
           result,
           fn.name,
           start,

test/parallel/test-perf-hooks-timerify-async-throw.js

@@ -0,0 +1,104 @@
+// Regression test for https://github.com/nodejs/node/issues/42743
+// Test that a timerified function which throws asynchronously (i.e. returns
+// a rejected promise / thenable) behaves consistently with one that throws
+// synchronously: no 'function' performance entry is produced, and no
+// histogram record is added.
+
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const {
+  createHistogram,
+  performance,
+  PerformanceObserver,
+  timerify,
+} = require('perf_hooks');
+
+// 1. Sync vs async throw should record the same number of histogram samples.
+{
+  const h1 = createHistogram();
+  const h2 = createHistogram();
+
+  function syncThrow() { throw new Error('sync'); }
+
+  async function asyncThrow() { throw new Error('async'); }
+
+  const g1 = timerify(syncThrow, { histogram: h1 });
+  const g2 = timerify(asyncThrow, { histogram: h2 });
+
+  assert.throws(() => g1(), /^Error: sync$/);
+
+  assert.rejects(g2(), /^Error: async$/).then(common.mustCall(() => {
+    // Both histograms must agree: neither should have recorded a sample,
+    // because the throwing async function should behave like the throwing
+    // sync function.
+    assert.strictEqual(h1.count, 0);
+    assert.strictEqual(h2.count, 0);
+    assert.strictEqual(h1.count, h2.count);
+  }));
+}
+
+// 2. No 'function' PerformanceEntry should be produced for either path.
+{
+  const obs = new PerformanceObserver(common.mustNotCall(
+    'no function entry should be enqueued for a throwing timerified fn',
+  ));
+  obs.observe({ entryTypes: ['function'] });
+
+  const sync = timerify(() => { throw new Error('sync2'); });
+  const async_ = timerify(async () => { throw new Error('async2'); });
+
+  assert.throws(() => sync(), /^Error: sync2$/);
+  assert.rejects(async_(), /^Error: async2$/).then(common.mustCall(() => {
+    // Give the observer a tick to flush any (incorrectly) enqueued entries
+    // before disconnecting.
+    setImmediate(common.mustCall(() => obs.disconnect()));
+  }));
+}
+
+// 3. A custom thenable that only implements `then` (no `finally`) and
+// fulfills should still produce a performance entry and resolve to the
+// original value. This guards against a regression where the wrapper
+// relied on `result.finally`.
+{
+  const obs = new PerformanceObserver(common.mustCall((list, observer) => {
+    const entries = list.getEntries();
+    assert.strictEqual(entries.length, 1);
+    const entry = entries[0];
+    assert.strictEqual(entry.entryType, 'function');
+    assert.strictEqual(typeof entry.duration, 'number');
+    assert.strictEqual(typeof entry.startTime, 'number');
+    observer.disconnect();
+  }));
+  obs.observe({ entryTypes: ['function'] });
+
+  function thenableOnly() {
+    return {
+      then(onFulfilled) {
+        // Resolve asynchronously to mimic real promise semantics.
+        setImmediate(() => onFulfilled('value'));
+      },
+    };
+  }
+
+  const wrapped = timerify(thenableOnly);
+  const ret = wrapped();
+  // The wrapper must return something thenable; `.then` must work and
+  // the resolved value must be propagated unchanged.
+  assert.strictEqual(typeof ret.then, 'function');
+  ret.then(common.mustCall((value) => {
+    assert.strictEqual(value, 'value');
+  }));
+}
+
+// 4. Async fulfillment must still propagate the resolved value.
+{
+  const wrapped = timerify(async () => 42);
+  wrapped().then(common.mustCall((value) => {
+    assert.strictEqual(value, 42);
+  }));
+}
+
+// Reference `performance` so the binding is exercised in this file too.
+assert.strictEqual(performance.timerify, timerify);