From a05a5a8735d117550f3c427d8aa8f8b23926f3c6 Mon Sep 17 00:00:00 2001 From: Igor Octaviano Date: Tue, 3 Nov 2026 18:58:12 -0300 Subject: [PATCH] fix: stop XHR retries once the request is aborted dicomweb-client passes the request's AbortSignal to request hooks as `metadata.signal` when the `request.signal` option is used. The retry hook now honors it: an aborted request is not sent, a scheduled retry is cancelled instead of re-opening and re-sending the request, and a retryable response that arrives after the abort is passed on rather than retried. Without a signal the hook behaves as before. This keeps cancelled tile requests (e.g. Viv tiles pruned while panning) from being downloaded again after a 429/5xx. --- src/AppConfig.d.ts | 5 ++ src/utils/__tests__/xhrRetryHook.test.ts | 75 ++++++++++++++++++++++++ src/utils/xhrRetryHook.ts | 15 ++++- 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/AppConfig.d.ts b/src/AppConfig.d.ts index 9e1d177c..701be160 100644 --- a/src/AppConfig.d.ts +++ b/src/AppConfig.d.ts @@ -11,6 +11,11 @@ export interface DICOMwebClientRequestHookMetadata { method: string /** Combined request headers from dicomweb-client (needed to re-apply after retry open()). */ headers?: Record + /** + * Signal of the request, passed by dicomweb-client versions that support + * the `request.signal` option. Hooks must not resend an aborted request. + */ + signal?: AbortSignal } export interface RetryRequestSettings { diff --git a/src/utils/__tests__/xhrRetryHook.test.ts b/src/utils/__tests__/xhrRetryHook.test.ts index 0f1176ec..c7713109 100644 --- a/src/utils/__tests__/xhrRetryHook.test.ts +++ b/src/utils/__tests__/xhrRetryHook.test.ts @@ -184,4 +184,79 @@ describe('getXHRRetryHook', () => { expect(laterWrapper).toHaveBeenCalledTimes(1) expect(clientHandler).not.toHaveBeenCalled() }) + + describe('with an abort signal', () => { + /** Retry delay long enough to abort while a retry is scheduled. */ + const slowRetryOptions = { + ...fastRetryOptions, + minTimeout: 30, + maxTimeout: 30, + } + + const applyHookWithSignal = (xhr: FakeXHR, signal: AbortSignal): void => { + getXHRRetryHook(slowRetryOptions)(xhr as unknown as XMLHttpRequest, { + url: 'https://example.com/studies', + method: 'GET', + headers: { Accept: 'application/dicom+json' }, + signal, + }) + } + + it('does not re-send when aborted while a retry is scheduled', async () => { + const xhr = new FakeXHR() + xhr.statusQueue = [500, 200] + const controller = new AbortController() + applyHookWithSignal(xhr, controller.signal) + + xhr.send() + /** First response arrived; the retry is waiting for its timeout. */ + await flush(5) + controller.abort() + await flush(100) + + expect(xhr.sendCount).toBe(1) + expect(xhr.openCalls).toEqual([]) + }) + + it('does not retry a response that arrives after the abort', async () => { + const xhr = new FakeXHR() + xhr.statusQueue = [503, 200] + const clientHandler = jest.fn() + xhr.onreadystatechange = clientHandler + const controller = new AbortController() + applyHookWithSignal(xhr, controller.signal) + + xhr.send() + controller.abort() + await flush(100) + + expect(xhr.sendCount).toBe(1) + expect(clientHandler).toHaveBeenCalledTimes(1) + }) + + it('does not send when the signal is already aborted', async () => { + const xhr = new FakeXHR() + const controller = new AbortController() + controller.abort() + applyHookWithSignal(xhr, controller.signal) + + xhr.send() + await flush() + + expect(xhr.sendCount).toBe(0) + }) + + it('stops listening to the signal once the request has finished', async () => { + const xhr = new FakeXHR() + xhr.statusQueue = [200] + const controller = new AbortController() + const removeSpy = jest.spyOn(controller.signal, 'removeEventListener') + applyHookWithSignal(xhr, controller.signal) + + xhr.send() + await flush() + + expect(removeSpy).toHaveBeenCalledWith('abort', expect.any(Function)) + }) + }) }) diff --git a/src/utils/xhrRetryHook.ts b/src/utils/xhrRetryHook.ts index 057fcd00..c439837e 100644 --- a/src/utils/xhrRetryHook.ts +++ b/src/utils/xhrRetryHook.ts @@ -58,13 +58,15 @@ export const getXHRRetryHook = ( * @param metadata - Metadata about the request * @param metadata.url - URL * @param metadata.method - HTTP method + * @param metadata.signal - Signal of the request; once it is aborted, no + * further attempts are made * @returns - XHR request instance (potentially modified) */ return ( request: XMLHttpRequest, metadata: DICOMwebClientRequestHookMetadata, ): XMLHttpRequest => { - const { url, method } = metadata + const { url, method, signal } = metadata if (!RETRYABLE_METHODS.has(method.toUpperCase())) { return request @@ -93,7 +95,16 @@ export const getXHRRetryHook = ( randomize: retryOptions.randomize, }) + /** Cancel a scheduled retry when the request is aborted while waiting */ + const stopOnAbort = (): void => { + operation.stop() + } + signal?.addEventListener('abort', stopOnAbort, { once: true }) + operation.attempt(function operationAttempt(currentAttempt) { + if (signal?.aborted === true) { + return + } if (currentAttempt > 1) { console.warn(`Requesting ${url}... (attempt: ${currentAttempt})`) /** open() empties author request headers; re-apply those + responseType. */ @@ -112,6 +123,7 @@ export const getXHRRetryHook = ( } if ( + signal?.aborted !== true && retryOptions.retryableStatusCodes.includes(request.status) && operation.retry( new Error( @@ -123,6 +135,7 @@ export const getXHRRetryHook = ( return } + signal?.removeEventListener('abort', stopOnAbort) if (downstreamOnReadyStateChange != null) { downstreamOnReadyStateChange.call(request, ev) }