The coalescing transport no longer double-throws AbortError when th…

…e last subscriber aborts (#2029)
solana-labs · Jan 12, 2024 · dbe6a73 · dbe6a73
1 parent 62b6bd6
commit dbe6a73
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 14 deletions.
diff --git a/packages/library/src/__tests__/rpc-request-coalescer-test.ts b/packages/library/src/__tests__/rpc-request-coalescer-test.ts
@@ -9,7 +9,7 @@ describe('RPC request coalescer', () => {
     beforeEach(() => {
         jest.useFakeTimers();
         hashFn = jest.fn();
-        mockTransport = jest.fn();
+        mockTransport = jest.fn().mockResolvedValue(null);
         coalescedTransport = getRpcTransportWithRequestCoalescing(mockTransport, hashFn);
     });
     describe('when requests produce the same hash', () => {
@@ -88,12 +88,16 @@ describe('RPC request coalescer', () => {
             beforeEach(() => {
                 abortControllerA = new AbortController();
                 abortControllerB = new AbortController();
-                mockTransport.mockImplementation(
-                    () =>
-                        new Promise(resolve => {
-                            transportResponsePromise = resolve;
-                        }),
-                );
+                mockTransport.mockImplementation(async ({ signal }) => {
+                    signal?.throwIfAborted();
+                    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);
+                        });
+                    });
+                });
                 responsePromiseA = coalescedTransport({ payload: null, signal: abortControllerA.signal });
                 responsePromiseB = coalescedTransport({ payload: null, signal: abortControllerB.signal });
             });
@@ -110,13 +114,12 @@ describe('RPC request coalescer', () => {
                 abortControllerA.abort('o no');
                 await expect(responsePromiseA).rejects.toThrow(/o no/);
             });
-            it('aborts the transport when all of the requests abort', async () => {
-                expect.assertions(1);
+            it('aborts the transport when all of the requests abort', () => {
                 abortControllerA.abort('o no A');
                 abortControllerB.abort('o no B');
                 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
                 const transportAbortSignal = mockTransport.mock.lastCall![0].signal!;
-                await expect(transportAbortSignal.aborted).toBe(true);
+                expect(transportAbortSignal.aborted).toBe(true);
             });
             it('does not abort the transport if fewer than every request aborts', async () => {
                 expect.assertions(1);

diff --git a/packages/library/src/rpc-request-coalescer.ts b/packages/library/src/rpc-request-coalescer.ts
@@ -29,13 +29,28 @@ export function getRpcTransportWithRequestCoalescing(
         }
         if (coalescedRequestsByDeduplicationKey[deduplicationKey] == null) {
             const abortController = new AbortController();
+            const responsePromise = (async () => {
+                try {
+                    return await transport<TResponse>({
+                        ...config,
+                        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.
+                        return;
+                    }
+                    throw e;
+                }
+            })();
             coalescedRequestsByDeduplicationKey[deduplicationKey] = {
                 abortController,
                 numConsumers: 0,
-                responsePromise: transport<TResponse>({
-                    ...config,
-                    signal: abortController.signal,
-                }),
+                responsePromise,
             };
         }
         const coalescedRequest = coalescedRequestsByDeduplicationKey[deduplicationKey];