Skip to content

Commit c471d36

Browse files
authored
fix(diagnostics): emit asyncStart after promise settlement to match native tracePromise (#4824)
Previously, `asyncStart` was emitted when the promise was first returned, rather than when the promise settled. Emitting it early meant subscribers that temporarily activate context between asyncStart and asyncEnd could leak that context into overlapping sibling operations. This PR moves asyncStart to the start of the continuing callback, matching the docs at https://nodejs.org/api/diagnostics_channel.html#asyncstartevent: >The asyncStart event represents the callback or continuation of a traceable function being reached. At this point things like callback arguments may be available, or anything else expressing the "result" of the action. This did not show up in earlier tests because most subscribers only used `start` for context creation and `asyncEnd` for completion. This should not be a breaking change because the terminal `end/asyncEnd` semantics and result/error payloads remain the same. This is an invisible change for most users, and is only correcting a slight mismatch. We had to manually implement the async lifecycle in `traceMixed` here because we needed to preserve the `sync/async` return paths in resolvers.
1 parent 3f3895f commit c471d36

4 files changed

Lines changed: 14 additions & 5 deletions

File tree

src/diagnostics.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,17 +427,18 @@ export function traceMixed<TResult, TContext extends TraceLifecycleContext>(
427427
}
428428

429429
channel.end.publish(context);
430-
channel.asyncStart.publish(context);
431430

432431
return result.then(
433432
(value) => {
434433
context.result = value;
434+
channel.asyncStart.publish(context);
435435
channel.asyncEnd.publish(context);
436436
return value;
437437
},
438438
(err: unknown) => {
439439
context.error = err;
440440
channel.error.publish(context);
441+
channel.asyncStart.publish(context);
441442
channel.asyncEnd.publish(context);
442443
throw err;
443444
},

src/execution/__tests__/diagnostics-execute-test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ describe('execute diagnostics channel', () => {
121121
rawVariableValues: undefined,
122122
operationName: undefined,
123123
operationType: OperationTypeNode.QUERY,
124+
result,
124125
},
125126
},
126127
{
@@ -201,17 +202,18 @@ describe('execute diagnostics channel', () => {
201202
},
202203
},
203204
{
204-
channel: 'asyncStart',
205+
channel: 'error',
205206
context: {
206207
schema: asyncDeferSchema,
207208
document,
208209
rawVariableValues: undefined,
209210
operationName: 'Deferred',
210211
operationType: OperationTypeNode.QUERY,
212+
error,
211213
},
212214
},
213215
{
214-
channel: 'error',
216+
channel: 'asyncStart',
215217
context: {
216218
schema: asyncDeferSchema,
217219
document,
@@ -456,6 +458,7 @@ describe('execute root selection set diagnostics channel', () => {
456458
rawVariableValues: undefined,
457459
operationName: undefined,
458460
operationType: OperationTypeNode.QUERY,
461+
result,
459462
},
460463
},
461464
{

src/execution/__tests__/diagnostics-resolve-test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ describe('resolve diagnostics channel', () => {
130130
args: {},
131131
isDefaultResolver: true,
132132
fieldPath: 'async',
133+
result: 'hello-async',
133134
},
134135
},
135136
{
@@ -207,6 +208,7 @@ describe('resolve diagnostics channel', () => {
207208
args: {},
208209
isDefaultResolver: true,
209210
fieldPath: 'async',
211+
result: 'hello-thenable',
210212
},
211213
},
212214
{
@@ -325,7 +327,7 @@ describe('resolve diagnostics channel', () => {
325327
},
326328
},
327329
{
328-
channel: 'asyncStart',
330+
channel: 'error',
329331
context: {
330332
fieldName: 'asyncFail',
331333
alias: 'asyncFail',
@@ -334,10 +336,11 @@ describe('resolve diagnostics channel', () => {
334336
args: {},
335337
isDefaultResolver: true,
336338
fieldPath: 'asyncFail',
339+
error,
337340
},
338341
},
339342
{
340-
channel: 'error',
343+
channel: 'asyncStart',
341344
context: {
342345
fieldName: 'asyncFail',
343346
alias: 'asyncFail',

src/execution/__tests__/diagnostics-subscribe-test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ describe('subscribe diagnostics channel', () => {
145145
rawVariableValues: undefined,
146146
operationName: undefined,
147147
operationType: OperationTypeNode.SUBSCRIPTION,
148+
result,
148149
},
149150
},
150151
{
@@ -474,6 +475,7 @@ describe('subscribe diagnostics channel', () => {
474475
rawVariableValues: undefined,
475476
operationName: 'S',
476477
operationType: OperationTypeNode.SUBSCRIPTION,
478+
result,
477479
},
478480
},
479481
{

0 commit comments

Comments
 (0)