Skip to content

Commit 74fdca6

Browse files
committed
withConcurrentAbruptClose should await handlers (#4532)
this makes our helper function more robust/predictable
1 parent 493f428 commit 74fdca6

3 files changed

Lines changed: 106 additions & 5 deletions

File tree

src/execution/__tests__/mapAsyncIterable-test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

44
import { expectPromise } from '../../__testUtils__/expectPromise.js';
5+
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
56

67
import { mapAsyncIterable } from '../mapAsyncIterable.js';
78

@@ -248,6 +249,66 @@ describe('mapAsyncIterable', () => {
248249
});
249250
});
250251

252+
it('waits for source handlers before actually throwing', async () => {
253+
const abortReason = new Error('aborted');
254+
let storedReason: unknown;
255+
256+
const iterable: AsyncIterableIterator<number> = {
257+
[Symbol.asyncIterator]() {
258+
return this;
259+
},
260+
next() {
261+
return Promise.resolve({ value: 1, done: false });
262+
},
263+
async throw(reason?: unknown) {
264+
if (storedReason === undefined) {
265+
await resolveOnNextTick();
266+
// eslint-disable-next-line require-atomic-updates
267+
storedReason = reason;
268+
return { value: undefined, done: true };
269+
}
270+
// eslint-disable-next-line @typescript-eslint/only-throw-error
271+
throw storedReason;
272+
},
273+
return() {
274+
return Promise.resolve({ value: undefined, done: true });
275+
},
276+
};
277+
278+
const mapped = mapAsyncIterable(iterable, (x) => x);
279+
280+
expect(await mapped.next()).to.deep.equal({ value: 1, done: false });
281+
282+
const thrown = mapped.throw(abortReason);
283+
await expectPromise(thrown).toRejectWith('aborted');
284+
expect(storedReason).to.equal(abortReason);
285+
});
286+
287+
it('throws given reason, ignoring source throw result', async () => {
288+
const iterable: AsyncIterableIterator<number> = {
289+
[Symbol.asyncIterator]() {
290+
return this;
291+
},
292+
next() {
293+
return Promise.resolve({ value: 1, done: false });
294+
},
295+
throw(_reason?: unknown) {
296+
return Promise.resolve({ value: 1, done: false });
297+
},
298+
return() {
299+
return Promise.resolve({ value: undefined, done: true });
300+
},
301+
};
302+
303+
const mapped = mapAsyncIterable(iterable, (x) => x);
304+
305+
expect(await mapped.next()).to.deep.equal({ value: 1, done: false });
306+
307+
const abortReason = new Error('aborted');
308+
const thrown = mapped.throw(abortReason);
309+
await expectPromise(thrown).toRejectWith('aborted');
310+
});
311+
251312
it('passes through caught errors through async generators', async () => {
252313
async function* source() {
253314
yield 1;

src/execution/__tests__/withConcurrentAbruptClose-test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,42 @@ describe('withConcurrentAbruptClose', () => {
175175

176176
expect(generator[Symbol.asyncIterator]()).to.equal(generator);
177177
});
178+
179+
it('awaits beforeThrow so an abrupt close can set the rejection reason', async () => {
180+
const abortReason = new Error('aborted');
181+
let storedReason: unknown;
182+
183+
const generator = {
184+
[Symbol.asyncIterator]() {
185+
return this;
186+
},
187+
next() {
188+
return Promise.resolve({ value: undefined, done: true });
189+
},
190+
throw() {
191+
return storedReason === undefined
192+
? Promise.resolve({ value: undefined, done: true })
193+
: // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors
194+
Promise.reject(storedReason);
195+
},
196+
return() {
197+
return Promise.resolve({ value: undefined, done: true });
198+
},
199+
async [Symbol.asyncDispose]() {
200+
await this.return();
201+
},
202+
};
203+
204+
const wrapped = withConcurrentAbruptClose(
205+
generator,
206+
() => undefined,
207+
async () => {
208+
await Promise.resolve();
209+
storedReason = abortReason;
210+
},
211+
);
212+
213+
await expectPromise(wrapped.throw(abortReason)).toRejectWith('aborted');
214+
expect(storedReason).to.equal(abortReason);
215+
});
178216
});

src/execution/withConcurrentAbruptClose.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,27 +32,29 @@ export function withConcurrentAbruptClose<T>(
3232
return generator.next();
3333
},
3434
async return() {
35-
ignoreErrors(beforeReturn);
35+
await ignoreErrors(beforeReturn);
3636
return generator.return();
3737
},
3838
async throw(error?: unknown) {
39-
ignoreErrors(() => beforeThrow(error));
39+
await ignoreErrors(() => beforeThrow(error));
4040
return generator.throw(error);
4141
},
4242
async [asyncDispose]() {
43-
ignoreErrors(beforeReturn);
43+
await ignoreErrors(beforeReturn);
4444
if (typeof generator[asyncDispose] === 'function') {
4545
await generator[asyncDispose]();
4646
}
4747
},
4848
};
4949
}
5050

51-
function ignoreErrors(fn: () => PromiseOrValue<unknown>): void {
51+
function ignoreErrors(
52+
fn: () => PromiseOrValue<unknown>,
53+
): PromiseOrValue<unknown> {
5254
try {
5355
const result = fn();
5456
if (isPromise(result)) {
55-
result.catch(() => {
57+
return result.catch(() => {
5658
// ignore error
5759
});
5860
}

0 commit comments

Comments
 (0)