Skip to content

Commit 4dc9171

Browse files
committed
feat(execution): expose async-work-finished execution hook
expose an execution hook that runs once tracked async work has finished, including work that can outlive a returned execution result The hook is available through `hooks.asyncWorkFinished` on execution args. `AsyncWorkTracker.wait()` now has a synchronous fast-path: - returns `void` when no async work is currently tracked - returns `Promise<void>` only while tracked work is pending This keeps synchronous execution paths synchronous when hooks are enabled. `GraphQLResolveInfoHelpers.promiseAll` is documented as a tracked helper that is intended to be returned/awaited as part of resolver work. Example (synchronous execution stays synchronous): ```ts const calls: Array<string> = []; const result = execute({ schema, document: parse('{ test }'), hooks: { asyncWorkFinished() { calls.push('asyncWorkFinished'); }, }, }); // result is sync and the hook has already run. console.log(result); console.log(calls); ``` Anti-pattern example (fire-and-forget `promiseAll` side effect): ```ts resolve(_source, _args, _context, info) { info .getAsyncHelpers() .promiseAll([Promise.reject(new Error('bad')), pendingCleanup]) .catch(() => undefined); return 'ok'; } ``` This pattern is discouraged. Tracking for this call starts on a later microtask, so `hooks.asyncWorkFinished` is not guaranteed to wait for it. A regression test now asserts this behavior. Recommended fire-and-forget pattern: ```ts resolve(_source, _args, _context, info) { const cleanup = doCleanupAsync(); info.getAsyncHelpers().trackPromise(cleanup); return 'ok'; } ```
1 parent 1e9a455 commit 4dc9171

File tree

13 files changed

+676
-11
lines changed

13 files changed

+676
-11
lines changed

src/execution/AsyncWorkTracker.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,15 @@ export class AsyncWorkTracker {
3030
}
3131
}
3232

33+
wait(): Promise<void> | void {
34+
// wait can complete synchronously when there is no tracked async work,
35+
// which allows synchronous execution paths to remain synchronous.
36+
if (this.pendingAsyncWork.size === 0) {
37+
return;
38+
}
39+
return this.waitForPendingAsyncWork();
40+
}
41+
3342
promiseAllTrackOnReject<T>(
3443
values: ReadonlyArray<PromiseOrValue<T>>,
3544
): Promise<Array<T>> {
@@ -39,4 +48,21 @@ export class AsyncWorkTracker {
3948
});
4049
return promise;
4150
}
51+
52+
promiseCombinatorWithTracking<T, TResult>(
53+
values: ReadonlyArray<PromiseOrValue<T>>,
54+
combinator: (
55+
promises: ReadonlyArray<PromiseOrValue<T>>,
56+
) => Promise<TResult>,
57+
): Promise<TResult> {
58+
this.add(Promise.allSettled(values));
59+
return combinator(values);
60+
}
61+
62+
private async waitForPendingAsyncWork(): Promise<void> {
63+
while (this.pendingAsyncWork.size > 0) {
64+
// eslint-disable-next-line no-await-in-loop
65+
await Promise.allSettled(Array.from(this.pendingAsyncWork));
66+
}
67+
}
4268
}

src/execution/Executor.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ import { createSharedExecutionContext } from './createSharedExecutionContext.js'
6161
import { buildResolveInfo } from './execute.js';
6262
import type { StreamUsage } from './getStreamUsage.js';
6363
import { getStreamUsage as _getStreamUsage } from './getStreamUsage.js';
64+
import type { ExecutionHooks } from './hooks.js';
65+
import { runAsyncWorkFinishedHook } from './hooks.js';
6466
import { returnIteratorCatchingErrors } from './returnIteratorCatchingErrors.js';
6567
import type { VariableValues } from './values.js';
6668
import { getArgumentValues } from './values.js';
@@ -116,6 +118,7 @@ export interface ValidatedExecutionArgs {
116118
errorPropagation: boolean;
117119
externalAbortSignal: AbortSignal | undefined;
118120
enableEarlyExecution: boolean;
121+
hooks: ExecutionHooks | undefined;
119122
}
120123

121124
/**
@@ -372,17 +375,31 @@ export class Executor<
372375
this.aborted = true;
373376
}
374377

378+
finishSharedExecution(): void {
379+
this.resolverAbortController?.abort();
380+
const asyncWorkFinishedHook =
381+
this.validatedExecutionArgs.hooks?.asyncWorkFinished;
382+
if (asyncWorkFinishedHook === undefined) {
383+
return;
384+
}
385+
runAsyncWorkFinishedHook(
386+
this.validatedExecutionArgs,
387+
this.sharedExecutionContext,
388+
asyncWorkFinishedHook,
389+
);
390+
}
391+
375392
/**
376393
* Given a completed execution context and data, build the `{ errors, data }`
377394
* response defined by the "Response" section of the GraphQL specification.
378395
*/
379396
buildResponse(
380397
data: ObjMap<unknown> | null,
381398
): ExecutionResult | TAlternativeInitialResponse {
399+
this.finishSharedExecution();
382400
this.finish();
383401
const errors = this.collectedErrors.errors;
384402
const result = errors.length ? { errors, data } : { data };
385-
this.resolverAbortController?.abort();
386403
return result;
387404
}
388405

src/execution/__tests__/AsyncWorkTracker-test.ts

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe('promiseAllTrackOnReject', () => {
5858
expect(settled).to.equal(true);
5959
});
6060

61-
it('tracks all promises only after rejection', async () => {
61+
it('tracks sibling promises only after rejection', async () => {
6262
const delayed = promiseWithResolvers<undefined>();
6363
const tracker = new AsyncWorkTracker();
6464
const result = tracker.promiseAllTrackOnReject([
@@ -103,3 +103,125 @@ describe('promiseAllTrackOnReject', () => {
103103
expect(unhandledRejection).to.equal(null);
104104
});
105105
});
106+
107+
describe('promiseCombinatorWithTracking', () => {
108+
it('tracks pending promises after Promise.any resolves', async () => {
109+
const tracker = new AsyncWorkTracker();
110+
111+
const delayed = promiseWithResolvers<number>();
112+
const result = await tracker.promiseCombinatorWithTracking(
113+
[1, delayed.promise],
114+
// eslint-disable-next-line @typescript-eslint/await-thenable
115+
(promises) => Promise.any(promises),
116+
);
117+
expect(result).to.equal(1);
118+
119+
await resolveOnNextTick();
120+
expect(tracker.pendingAsyncWork.size).to.equal(1);
121+
122+
delayed.resolve(2);
123+
await Promise.allSettled(Array.from(tracker.pendingAsyncWork));
124+
await resolveOnNextTick();
125+
expect(tracker.pendingAsyncWork.size).to.equal(0);
126+
});
127+
128+
it('tracks pending promises after Promise.race rejects', async () => {
129+
const tracker = new AsyncWorkTracker();
130+
131+
const delayed = promiseWithResolvers<number>();
132+
const result = tracker.promiseCombinatorWithTracking(
133+
[Promise.reject(new Error('bad')), delayed.promise],
134+
// eslint-disable-next-line @typescript-eslint/await-thenable
135+
(promises) => Promise.race(promises),
136+
);
137+
138+
await result.catch(() => undefined);
139+
await resolveOnNextTick();
140+
expect(tracker.pendingAsyncWork.size).to.equal(1);
141+
142+
delayed.resolve(1);
143+
await Promise.allSettled(Array.from(tracker.pendingAsyncWork));
144+
await resolveOnNextTick();
145+
expect(tracker.pendingAsyncWork.size).to.equal(0);
146+
});
147+
});
148+
149+
describe('wait', () => {
150+
it('returns synchronously when there is no pending async work', () => {
151+
const tracker = new AsyncWorkTracker();
152+
expect(tracker.wait()).to.equal(undefined);
153+
});
154+
155+
it('waits when tracked async work is present', async () => {
156+
const tracker = new AsyncWorkTracker();
157+
158+
const delayed = promiseWithResolvers<undefined>();
159+
tracker.add(delayed.promise);
160+
161+
let settled = false;
162+
const maybeWait = tracker.wait();
163+
expect(maybeWait).to.be.instanceOf(Promise);
164+
const wait = Promise.resolve(maybeWait).then(() => {
165+
settled = true;
166+
});
167+
await resolveOnNextTick();
168+
expect(settled).to.equal(false);
169+
170+
delayed.resolve(undefined);
171+
await wait;
172+
expect(settled).to.equal(true);
173+
});
174+
175+
it('keeps waiting when tracked async work is followed by more tracked async work', async () => {
176+
const tracker = new AsyncWorkTracker();
177+
178+
const delayed = promiseWithResolvers<undefined>();
179+
tracker.add(delayed.promise);
180+
181+
let settled = false;
182+
const maybeWait = tracker.wait();
183+
expect(maybeWait).to.be.instanceOf(Promise);
184+
const wait = Promise.resolve(maybeWait).then(() => {
185+
settled = true;
186+
});
187+
await resolveOnNextTick();
188+
expect(settled).to.equal(false);
189+
190+
delayed.resolve(undefined);
191+
192+
const anotherDelayed = promiseWithResolvers<undefined>();
193+
tracker.add(anotherDelayed.promise);
194+
await resolveOnNextTick();
195+
expect(settled).to.equal(false);
196+
197+
anotherDelayed.resolve(undefined);
198+
199+
await wait;
200+
expect(settled).to.equal(true);
201+
});
202+
203+
it('does not wait for side-effect promiseAll tracking added on next microtask', async () => {
204+
const tracker = new AsyncWorkTracker();
205+
const delayed = promiseWithResolvers<undefined>();
206+
207+
tracker
208+
.promiseAllTrackOnReject([
209+
Promise.reject(new Error('bad')),
210+
delayed.promise,
211+
])
212+
.catch(() => undefined);
213+
214+
const maybeWait = tracker.wait();
215+
expect(maybeWait).to.equal(undefined);
216+
217+
await resolveOnNextTick();
218+
expect(tracker.pendingAsyncWork.size).to.equal(0);
219+
await resolveOnNextTick();
220+
expect(tracker.pendingAsyncWork.size).to.be.greaterThan(0);
221+
222+
delayed.resolve(undefined);
223+
await Promise.allSettled(Array.from(tracker.pendingAsyncWork));
224+
await resolveOnNextTick();
225+
expect(tracker.pendingAsyncWork.size).to.equal(0);
226+
});
227+
});

src/execution/__tests__/executor-test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,12 @@ describe('Execute: Handles basic execution tasks', () => {
261261
'getAsyncHelpers',
262262
);
263263
const asyncHelpers = resolvedInfo?.getAsyncHelpers();
264-
expect(asyncHelpers).to.have.all.keys('trackPromise');
264+
expect(asyncHelpers).to.have.all.keys(
265+
'promiseAll',
266+
'promiseAny',
267+
'promiseRace',
268+
'trackPromise',
269+
);
265270

266271
const operation = document.definitions[0];
267272
assert(operation.kind === Kind.OPERATION_DEFINITION);
@@ -300,6 +305,18 @@ describe('Execute: Handles basic execution tasks', () => {
300305

301306
expect(resolvedInfo?.getAsyncHelpers()).to.equal(asyncHelpers);
302307

308+
const promiseAll = asyncHelpers?.promiseAll;
309+
expect(promiseAll).to.be.a('function');
310+
expect(resolvedInfo?.getAsyncHelpers().promiseAll).to.equal(promiseAll);
311+
312+
const promiseAny = asyncHelpers?.promiseAny;
313+
expect(promiseAny).to.be.a('function');
314+
expect(resolvedInfo?.getAsyncHelpers().promiseAny).to.equal(promiseAny);
315+
316+
const promiseRace = asyncHelpers?.promiseRace;
317+
expect(promiseRace).to.be.a('function');
318+
expect(resolvedInfo?.getAsyncHelpers().promiseRace).to.equal(promiseRace);
319+
303320
const trackPromise = asyncHelpers?.trackPromise;
304321
expect(trackPromise).to.be.a('function');
305322
expect(resolvedInfo?.getAsyncHelpers().trackPromise).to.equal(trackPromise);

0 commit comments

Comments
 (0)