Skip to content

Conversation

@Renegade334
Copy link
Member

tracingChannel.tracePromise() is documented in the source code (albeit not in the docs) to accept functions that return non-Promise thenables. However, it currently has some wonky behaviour when the return value isn't a native Promise, and also in the undocumented case where the return value isn't promise-like at all.

What gets returned by tracePromise() is highly dependent on whether the tracing channel has active subscribers or not:

result type Without subscribers With subscribers
Return value Return type Return value Return type
Native Promise result Native Promise result.then(...) Native Promise
Custom thenable result Custom thenable Promise.resolve(result).then(...) Native Promise
Non-thenable result Non-thenable Promise.resolve(result).then(...) Native Promise

The major inconsistencies are:

  • If the wrapped function returns a custom thenable, then it will either be returned as a custom thenable or wrapped in a native Promise, depending on whether there are active subscribers.
  • If the wrapped function returns a non-thenable, then it will either be returned verbatim or wrapped in a resolved native Promise, depending on whether there are active subscribers.
  • In the case where the wrapped function synchronously returns a non-thenable, then tracePromise produces asyncStart/asyncEnd events, despite no asynchronous execution. On the other hand, if the wrapped function throws synchronously, these events are not produced.

This PR makes some changes to the non-Promise cases to make the behaviours consistent.

  • If the wrapped function returns either a Promise or a custom thenable, then the value returned by tracePromise will have the same type: either the original value (if no subscribers) or the value of result.then(...) (if active subscribers).
  • If the wrapped function returns a non-thenable result, then it is returned verbatim, and a warning is emitted. No asyncStart or asyncEnd events are produced.
result type Without subscribers With subscribers
Return value Return type Return value Return type
Native Promise result Native Promise result.then(...) Native Promise
Custom thenable result Custom thenable result.then(...) Custom thenable
Non-thenable result Non-thenable result Non-thenable

Fixes: #59936

@nodejs-github-bot nodejs-github-bot added diagnostics_channel Issues and PRs related to diagnostics channel needs-ci PRs that need a full CI run. labels Feb 10, 2026
// TODO: Is there a way to have asyncEnd _after_ the continuation?
asyncEnd.publish(context);
return PromiseReject(err);
throw err;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing here avoids Promisifying thenables on rejection. The Promises/A+ standard dictates that throwing within the promise handlers results in the returned thenable rejecting. (https://promisesaplus.com/#point-42)

For native Promises, there should be no observable change.

@Renegade334 Renegade334 added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 10, 2026
@Renegade334 Renegade334 force-pushed the tracepromise-custom-thenables branch from b54c7f4 to 71a5eb2 Compare February 10, 2026 21:40
@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.75%. Comparing base (3819c7f) to head (71a5eb2).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
lib/diagnostics_channel.js 84.21% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #61766   +/-   ##
=======================================
  Coverage   89.74%   89.75%           
=======================================
  Files         675      675           
  Lines      204642   204650    +8     
  Branches    39322    39325    +3     
=======================================
+ Hits       183657   183683   +26     
+ Misses      13257    13251    -6     
+ Partials     7728     7716   -12     
Files with missing lines Coverage Δ
lib/diagnostics_channel.js 98.45% <84.21%> (-0.65%) ⬇️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diagnostics_channel Issues and PRs related to diagnostics channel needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tracingChannel.tracePromise forces native promises

2 participants