| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent a62f641 commit b4ea323
4 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -829,23 +829,36 @@ channels.traceSync(() => { | |||
| 829 | 829 | added: | |
| 830 | 830 | - v19.9.0 | |
| 831 | 831 | - v18.19.0 | |
| 832 | + changes: | ||
| 833 | + - version: REPLACEME | ||
| 834 | + pr-url: https://github.com/nodejs/node/pull/61766 | ||
| 835 | + description: Custom thenables will no longer be wrapped in native Promises. | ||
| 836 | + Non-thenables will be returned with a warning. | ||
| 832 | 837 | --> | |
| 833 | 838 | ||
| 834 | - * `fn` {Function} Promise-returning function to wrap a trace around | ||
| 839 | + * `fn` {Function} Function to wrap a trace around | ||
| 835 | 840 | * `context` {Object} Shared object to correlate trace events through | |
| 836 | 841 | * `thisArg` {any} The receiver to be used for the function call | |
| 837 | 842 | * `...args` {any} Optional arguments to pass to the function | |
| 838 | - * Returns: {Promise} Chained from promise returned by the given function | ||
| 839 | - | ||
| 840 | - Trace a promise-returning function call. This will always produce a | ||
| 841 | - [`start` event][] and [`end` event][] around the synchronous portion of the | ||
| 842 | - function execution, and will produce an [`asyncStart` event][] and | ||
| 843 | - [`asyncEnd` event][] when a promise continuation is reached. It may also | ||
| 844 | - produce an [`error` event][] if the given function throws an error or the | ||
| 845 | - returned promise rejects. This will run the given function using | ||
| 843 | + * Returns: {any} The return value of the given function, or the result of | ||
| 844 | + calling `.then(...)` on the return value if the tracing channel has active | ||
| 845 | + subscribers. If the return value is not a Promise or thenable, then | ||
| 846 | + it is returned as-is and a warning is emitted. | ||
| 847 | + | ||
| 848 | + Trace an asynchronous function call which returns a {Promise} or | ||
| 849 | + [thenable object][]. This will always produce a [`start` event][] and | ||
| 850 | + [`end` event][] around the synchronous portion of the function execution, and | ||
| 851 | + will produce an [`asyncStart` event][] and [`asyncEnd` event][] when the | ||
| 852 | + returned promise is resolved or rejected. It may also produce an | ||
| 853 | + [`error` event][] if the given function throws an error or the returned promise | ||
| 854 | + is rejected. This will run the given function using | ||
| 846 | 855 | [`channel.runStores(context, ...)`][] on the `start` channel which ensures all | |
| 847 | 856 | events should have any bound stores set to match this trace context. | |
| 848 | 857 | ||
| 858 | + If the value returned by `fn` is not a Promise or thenable, then it will be | ||
| 859 | + returned with a warning, and no `asyncStart` or `asyncEnd` events will be | ||
| 860 | + produced. | ||
| 861 | + | ||
| 849 | 862 | To ensure only correct trace graphs are formed, events will only be published | |
| 850 | 863 | if subscribers are present prior to starting the trace. Subscriptions which are | |
| 851 | 864 | added after the trace begins will not receive future events from that trace, | |
@@ -1526,3 +1539,4 @@ Emitted when a new thread is created. | |||
| 1526 | 1539 | [`start` event]: #startevent | |
| 1527 | 1540 | [`worker_threads.locks`]: worker_threads.md#worker_threadslocks | |
| 1528 | 1541 | [context loss]: async_context.md#troubleshooting-context-loss | |
| 1542 | + [thenable object]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#thenables | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -10,10 +10,6 @@ const { | |||
| 10 | 10 | ObjectDefineProperty, | |
| 11 | 11 | ObjectGetPrototypeOf, | |
| 12 | 12 | ObjectSetPrototypeOf, | |
| 13 | - Promise, | ||
| 14 | - PromisePrototypeThen, | ||
| 15 | - PromiseReject, | ||
| 16 | - PromiseResolve, | ||
| 17 | 13 | ReflectApply, | |
| 18 | 14 | SafeFinalizationRegistry, | |
| 19 | 15 | SafeMap, | |
@@ -292,6 +288,11 @@ function tracingChannelFrom(nameOrChannels, name) { | |||
| 292 | 288 | nameOrChannels); | |
| 293 | 289 | } | |
| 294 | 290 | ||
| 291 | + function emitNonThenableWarning(fn) { | ||
| 292 | + process.emitWarning(`tracePromise was called with the function '${fn.name || '<anonymous>'}', ` + | ||
| 293 | + 'which returned a non-thenable.'); | ||
| 294 | + } | ||
| 295 | + | ||
| 295 | 296 | class TracingChannel { | |
| 296 | 297 | constructor(nameOrChannels) { | |
| 297 | 298 | for (let i = 0; i < traceEvents.length; ++i) { | |
@@ -359,7 +360,11 @@ class TracingChannel { | |||
| 359 | 360 | ||
| 360 | 361 | tracePromise(fn, context = {}, thisArg, ...args) { | |
| 361 | 362 | if (!this.hasSubscribers) { | |
| 362 | - return ReflectApply(fn, thisArg, args); | ||
| 363 | + const result = ReflectApply(fn, thisArg, args); | ||
| 364 | + if (typeof result?.then !== 'function') { | ||
| 365 | + emitNonThenableWarning(fn); | ||
| 366 | + } | ||
| 367 | + return result; | ||
| 363 | 368 | } | |
| 364 | 369 | ||
| 365 | 370 | const { start, end, asyncStart, asyncEnd, error } = this; | |
@@ -370,7 +375,7 @@ class TracingChannel { | |||
| 370 | 375 | asyncStart.publish(context); | |
| 371 | 376 | // TODO: Is there a way to have asyncEnd _after_ the continuation? | |
| 372 | 377 | asyncEnd.publish(context); | |
| 373 | - return PromiseReject(err); | ||
| 378 | + throw err; | ||
| 374 | 379 | } | |
| 375 | 380 | ||
| 376 | 381 | function resolve(result) { | |
@@ -383,12 +388,15 @@ class TracingChannel { | |||
| 383 | 388 | ||
| 384 | 389 | return start.runStores(context, () => { | |
| 385 | 390 | try { | |
| 386 | - let promise = ReflectApply(fn, thisArg, args); | ||
| 387 | - // Convert thenables to native promises | ||
| 388 | - if (!(promise instanceof Promise)) { | ||
| 389 | - promise = PromiseResolve(promise); | ||
| 391 | + const result = ReflectApply(fn, thisArg, args); | ||
| 392 | + // If the return value is not a thenable, then return it with a warning. | ||
| 393 | + // Do not publish to asyncStart/asyncEnd. | ||
| 394 | + if (typeof result?.then !== 'function') { | ||
| 395 | + emitNonThenableWarning(fn); | ||
| 396 | + context.result = result; | ||
| 397 | + return result; | ||
| 390 | 398 | } | |
| 391 | - return PromisePrototypeThen(promise, resolve, reject); | ||
| 399 | + return result.then(resolve, reject); | ||
| 392 | 400 | } catch (err) { | |
| 393 | 401 | context.error = err; | |
| 394 | 402 | error.publish(context); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,46 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const common = require('../common'); | ||
| 4 | + const dc = require('diagnostics_channel'); | ||
| 5 | + const assert = require('assert'); | ||
| 6 | + | ||
| 7 | + const channel = dc.tracingChannel('test'); | ||
| 8 | + | ||
| 9 | + const expectedResult = { foo: 'bar' }; | ||
| 10 | + const input = { foo: 'bar' }; | ||
| 11 | + const thisArg = { baz: 'buz' }; | ||
| 12 | + | ||
| 13 | + function checkStart(found) { | ||
| 14 | + assert.strictEqual(found, input); | ||
| 15 | + } | ||
| 16 | + | ||
| 17 | + function checkEnd(found) { | ||
| 18 | + checkStart(found); | ||
| 19 | + assert.strictEqual(found.error, undefined); | ||
| 20 | + assert.deepStrictEqual(found.result, expectedResult); | ||
| 21 | + } | ||
| 22 | + | ||
| 23 | + const handlers = { | ||
| 24 | + start: common.mustCall(checkStart), | ||
| 25 | + end: common.mustCall(checkEnd), | ||
| 26 | + asyncStart: common.mustNotCall(), | ||
| 27 | + asyncEnd: common.mustNotCall(), | ||
| 28 | + error: common.mustNotCall() | ||
| 29 | + }; | ||
| 30 | + | ||
| 31 | + channel.subscribe(handlers); | ||
| 32 | + | ||
| 33 | + process.on('warning', common.mustCall((warning) => { | ||
| 34 | + assert.strictEqual( | ||
| 35 | + warning.message, | ||
| 36 | + "tracePromise was called with the function '<anonymous>', which returned a non-thenable." | ||
| 37 | + ); | ||
| 38 | + })); | ||
| 39 | + | ||
| 40 | + assert.strictEqual( | ||
| 41 | + channel.tracePromise(common.mustCall(function(value) { | ||
| 42 | + assert.deepStrictEqual(this, thisArg); | ||
| 43 | + return value; | ||
| 44 | + }), input, thisArg, expectedResult), | ||
| 45 | + expectedResult, | ||
| 46 | + ); | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,55 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const common = require('../common'); | ||
| 4 | + const dc = require('diagnostics_channel'); | ||
| 5 | + const assert = require('assert'); | ||
| 6 | + | ||
| 7 | + class ResolvedThenable { | ||
| 8 | + #result; | ||
| 9 | + constructor(value) { | ||
| 10 | + this.#result = value; | ||
| 11 | + } | ||
| 12 | + then(resolve) { | ||
| 13 | + return new ResolvedThenable(resolve(this.#result)); | ||
| 14 | + } | ||
| 15 | + } | ||
| 16 | + | ||
| 17 | + const channel = dc.tracingChannel('test'); | ||
| 18 | + | ||
| 19 | + const expectedResult = { foo: 'bar' }; | ||
| 20 | + const input = { foo: 'bar' }; | ||
| 21 | + const thisArg = { baz: 'buz' }; | ||
| 22 | + | ||
| 23 | + function check(found) { | ||
| 24 | + assert.strictEqual(found, input); | ||
| 25 | + } | ||
| 26 | + | ||
| 27 | + function checkAsync(found) { | ||
| 28 | + check(found); | ||
| 29 | + assert.strictEqual(found.error, undefined); | ||
| 30 | + assert.deepStrictEqual(found.result, expectedResult); | ||
| 31 | + } | ||
| 32 | + | ||
| 33 | + const handlers = { | ||
| 34 | + start: common.mustCall(check), | ||
| 35 | + end: common.mustCall(check), | ||
| 36 | + asyncStart: common.mustCall(checkAsync), | ||
| 37 | + asyncEnd: common.mustCall(checkAsync), | ||
| 38 | + error: common.mustNotCall() | ||
| 39 | + }; | ||
| 40 | + | ||
| 41 | + channel.subscribe(handlers); | ||
| 42 | + | ||
| 43 | + let innerThenable; | ||
| 44 | + | ||
| 45 | + const result = channel.tracePromise(common.mustCall(function(value) { | ||
| 46 | + assert.deepStrictEqual(this, thisArg); | ||
| 47 | + innerThenable = new ResolvedThenable(value); | ||
| 48 | + return innerThenable; | ||
| 49 | + }), input, thisArg, expectedResult); | ||
| 50 | + | ||
| 51 | + assert(result instanceof ResolvedThenable); | ||
| 52 | + assert.notStrictEqual(result, innerThenable); | ||
| 53 | + result.then(common.mustCall((value) => { | ||
| 54 | + assert.deepStrictEqual(value, expectedResult); | ||
| 55 | + })); | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments