Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
The request coalescer now throws the AbortSignal's reason (#2174)
Browse files Browse the repository at this point in the history
# Summary

I learned today that it's wrong to synthesize an `AbortError` `DOMException` with an `AbortSignal's` `reason`. You should throw the `reason` directly.

See https://fetch.spec.whatwg.org/#deserialize-a-serialized-abort-reason
  • Loading branch information
steveluscher authored Feb 27, 2024
1 parent 12eb0bf commit 356279e
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
16 changes: 11 additions & 5 deletions packages/rpc/src/__tests__/rpc-request-coalescer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ describe('RPC request coalescer', () => {
return await new Promise((resolve, reject) => {
transportResponsePromise = resolve;
signal?.addEventListener('abort', (e: AbortSignalEventMap['abort']) => {
const abortError = new DOMException((e.target as AbortSignal).reason, 'AbortError');
reject(abortError);
reject((e.target as AbortSignal).reason);
});
});
});
Expand All @@ -109,10 +108,17 @@ describe('RPC request coalescer', () => {
/* empty */
}
});
it('throws an `AbortError` from the aborted request', async () => {
it('throws an `AbortError` from the aborted request when no reason is specified', async () => {
expect.assertions(3);
abortControllerA.abort();
await expect(responsePromiseA).rejects.toThrow();
await expect(responsePromiseA).rejects.toBeInstanceOf(DOMException);
await expect(responsePromiseA).rejects.toHaveProperty('name', 'AbortError');
});
it("rejects from the aborted request with the `AbortSignal's` reason", async () => {
expect.assertions(1);
abortControllerA.abort('o no');
await expect(responsePromiseA).rejects.toThrow(/o no/);
await expect(responsePromiseA).rejects.toBe('o no');
});
it('aborts the transport when all of the requests abort', () => {
abortControllerA.abort('o no A');
Expand All @@ -134,7 +140,7 @@ describe('RPC request coalescer', () => {
const mockResponse = { response: 'ok' };
transportResponsePromise(mockResponse);
await Promise.all([
expect(responsePromiseA).rejects.toThrow(/o no A/),
expect(responsePromiseA).rejects.toBe('o no A'),
expect(responsePromiseB).resolves.toBe(mockResponse),
]);
});
Expand Down
22 changes: 13 additions & 9 deletions packages/rpc/src/rpc-request-coalescer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ type CoalescedRequest = {

type GetDeduplicationKeyFn = (payload: unknown) => string | undefined;

const EXPLICIT_ABORT_TOKEN = Symbol(
__DEV__
? 'This symbol is thrown from the request that underlies a series of coalesced requests ' +
'when the last request in that series aborts'
: undefined,
);

export function getRpcTransportWithRequestCoalescing<TTransport extends RpcTransport>(
transport: TTransport,
getDeduplicationKey: GetDeduplicationKeyFn,
Expand All @@ -34,12 +41,10 @@ export function getRpcTransportWithRequestCoalescing<TTransport extends RpcTrans
signal: abortController.signal,
});
} catch (e) {
if (e && typeof e === 'object' && 'name' in e && e.name === 'AbortError') {
// Ignore `AbortError` thrown from the underlying transport behind which all
// requests are coalesced. If it experiences an `AbortError` it is because
// we triggered one when the last subscriber aborted. Letting the underlying
// transport's `AbortError` bubble up from here would cause runtime fatals
// where there should be none.
if (e === EXPLICIT_ABORT_TOKEN) {
// We triggered this error when the last subscriber aborted. Letting this
// error bubble up from here would cause runtime fatals where there should
// be none.
return;
}
throw e;
Expand All @@ -61,10 +66,9 @@ export function getRpcTransportWithRequestCoalescing<TTransport extends RpcTrans
coalescedRequest.numConsumers -= 1;
if (coalescedRequest.numConsumers === 0) {
const abortController = coalescedRequest.abortController;
abortController.abort();
abortController.abort(EXPLICIT_ABORT_TOKEN);
}
const abortError = new DOMException((e.target as AbortSignal).reason, 'AbortError');
reject(abortError);
reject((e.target as AbortSignal).reason);
};
signal.addEventListener('abort', handleAbort);
responsePromise.then(resolve).finally(() => {
Expand Down

0 comments on commit 356279e

Please sign in to comment.