Tweaks to AbortSignal logic (#5150)

* avoid losing this when we call abort

* try simplifying timeout logic using ponyfills

* avoid breaking change

* make sure we always set once to true for abort listeners

* commit to technically breaking change

* avoid abortsignalwithreason now it's stable

* fix test expectations

* only use mocked fetch when needed

Author: EskiMojo14

packages/toolkit/src/createAsyncThunk.ts

@@ -651,7 +651,9 @@ export const createAsyncThunk = /* @__PURE__ */ (() => {
                   message: abortReason || 'Aborted',
                 })
               }
-              abortController.signal.addEventListener('abort', abortHandler)
+              abortController.signal.addEventListener('abort', abortHandler, {
+                once: true,
+              })
             })
             dispatch(
               pending(

packages/toolkit/src/listenerMiddleware/index.ts

@@ -19,7 +19,6 @@ import {
   validateActive,
 } from './task'
 import type {
-  AbortSignalWithReason,
   AddListenerOverloads,
   AnyListenerPredicate,
   CreateListenerMiddlewareOptions,
@@ -41,7 +40,6 @@ import type {
   UnsubscribeListenerOptions,
 } from './types'
 import {
-  abortControllerWithReason,
   addAbortSignalListener,
   assertFunction,
   catchRejection,
@@ -82,12 +80,12 @@ const INTERNAL_NIL_TOKEN = {} as const
 const alm = 'listenerMiddleware' as const
 
 const createFork = (
-  parentAbortSignal: AbortSignalWithReason<unknown>,
+  parentAbortSignal: AbortSignal,
   parentBlockingPromises: Promise<any>[],
 ) => {
   const linkControllers = (controller: AbortController) =>
     addAbortSignalListener(parentAbortSignal, () =>
-      abortControllerWithReason(controller, parentAbortSignal.reason),
+      controller.abort(parentAbortSignal.reason),
     )
 
   return <T>(
@@ -111,7 +109,7 @@ const createFork = (
         validateActive(childAbortController.signal)
         return result
       },
-      () => abortControllerWithReason(childAbortController, taskCompleted),
+      () => childAbortController.abort(taskCompleted),
     )
 
     if (opts?.autoJoin) {
@@ -121,7 +119,7 @@ const createFork = (
     return {
       result: createPause<TaskResult<T>>(parentAbortSignal)(result),
       cancel() {
-        abortControllerWithReason(childAbortController, taskCancelled)
+        childAbortController.abort(taskCancelled)
       },
     }
   }
@@ -256,7 +254,7 @@ const cancelActiveListeners = (
   entry: ListenerEntry<unknown, Dispatch<UnknownAction>>,
 ) => {
   entry.pending.forEach((controller) => {
-    abortControllerWithReason(controller, listenerCancelled)
+    controller.abort(listenerCancelled)
   })
 }
 
@@ -444,16 +442,13 @@ export const createListenerMiddleware = <
             cancelActiveListeners: () => {
               entry.pending.forEach((controller, _, set) => {
                 if (controller !== internalTaskController) {
-                  abortControllerWithReason(controller, listenerCancelled)
+                  controller.abort(listenerCancelled)
                   set.delete(controller)
                 }
               })
             },
             cancel: () => {
-              abortControllerWithReason(
-                internalTaskController,
-                listenerCancelled,
-              )
+              internalTaskController.abort(listenerCancelled)
               entry.pending.delete(internalTaskController)
             },
             throwIfCancelled: () => {
@@ -471,7 +466,7 @@ export const createListenerMiddleware = <
     } finally {
       await Promise.all(autoJoinPromises)
 
-      abortControllerWithReason(internalTaskController, listenerCompleted) // Notify that the task has completed
+      internalTaskController.abort(listenerCompleted) // Notify that the task has completed
       untrackExecutingListener(entry)
       entry.pending.delete(internalTaskController)
     }

packages/toolkit/src/listenerMiddleware/task.ts

@@ -1,17 +1,16 @@
 import { TaskAbortError } from './exceptions'
-import type { AbortSignalWithReason, TaskResult } from './types'
+import type { TaskResult } from './types'
 import { addAbortSignalListener, catchRejection, noop } from './utils'
 
 /**
  * Synchronously raises {@link TaskAbortError} if the task tied to the input `signal` has been cancelled.
  * @param signal
- * @param reason
  * @see {TaskAbortError}
+ * @throws {TaskAbortError} if the task tied to the input `signal` has been cancelled.
  */
 export const validateActive = (signal: AbortSignal): void => {
   if (signal.aborted) {
-    const { reason } = signal as AbortSignalWithReason<string>
-    throw new TaskAbortError(reason)
+    throw new TaskAbortError(signal.reason)
   }
 }
 
@@ -21,7 +20,7 @@ export const validateActive = (signal: AbortSignal): void => {
  * https://github.com/nodejs/node/issues/17469#issuecomment-349794909
  */
 export function raceWithSignal<T>(
-  signal: AbortSignalWithReason<string>,
+  signal: AbortSignal,
   promise: Promise<T>,
 ): Promise<T> {
   let cleanup = noop

packages/toolkit/src/listenerMiddleware/tests/fork.test.ts

@@ -2,11 +2,7 @@ import type { EnhancedStore } from '@reduxjs/toolkit'
 import { configureStore, createSlice, createAction } from '@reduxjs/toolkit'
 
 import type { PayloadAction } from '@reduxjs/toolkit'
-import type {
-  AbortSignalWithReason,
-  ForkedTaskExecutor,
-  TaskResult,
-} from '../types'
+import type { ForkedTaskExecutor, TaskResult } from '../types'
 import { createListenerMiddleware, TaskAbortError } from '../index'
 import {
   listenerCancelled,
@@ -382,9 +378,7 @@ describe('fork', () => {
             listenerApi.fork(
               async (forkApi) => {
                 forkApi.signal.addEventListener('abort', () => {
-                  deferredResult.resolve(
-                    (forkApi.signal as AbortSignalWithReason<unknown>).reason,
-                  )
+                  deferredResult.resolve(forkApi.signal.reason)
                 })
 
                 await forkApi.delay(10)

packages/toolkit/src/listenerMiddleware/tests/listenerMiddleware.test.ts

@@ -2,10 +2,7 @@ import {
   listenerCancelled,
   listenerCompleted,
 } from '@internal/listenerMiddleware/exceptions'
-import type {
-  AbortSignalWithReason,
-  AddListenerOverloads,
-} from '@internal/listenerMiddleware/types'
+import type { AddListenerOverloads } from '@internal/listenerMiddleware/types'
 import { noop } from '@internal/listenerMiddleware/utils'
 import type {
   Action,
@@ -606,7 +603,7 @@ describe('createListenerMiddleware', () => {
           signal.addEventListener(
             'abort',
             () => {
-              payload.resolve((signal as AbortSignalWithReason<string>).reason)
+              payload.resolve(signal.reason)
             },
             { once: true },
           )
@@ -636,7 +633,7 @@ describe('createListenerMiddleware', () => {
           signal.addEventListener(
             'abort',
             () => {
-              payload.resolve((signal as AbortSignalWithReason<string>).reason)
+              payload.resolve(signal.reason)
             },
             { once: true },
           )

packages/toolkit/src/listenerMiddleware/types.ts

@@ -10,12 +10,6 @@ import type { BaseActionCreator, PayloadAction } from '../createAction'
 import type { TypedActionCreator } from '../mapBuilders'
 import type { TaskAbortError } from './exceptions'
 
-/**
- * @internal
- * At the time of writing `lib.dom.ts` does not provide `abortSignal.reason`.
- */
-export type AbortSignalWithReason<T> = AbortSignal & { reason?: T }
-
 /**
  * Types copied from RTK
  */

packages/toolkit/src/listenerMiddleware/utils.ts

@@ -1,5 +1,3 @@
-import type { AbortSignalWithReason } from './types'
-
 export const assertFunction: (
   func: unknown,
   expected: string,
@@ -30,41 +28,3 @@ export const addAbortSignalListener = (
   abortSignal.addEventListener('abort', callback, { once: true })
   return () => abortSignal.removeEventListener('abort', callback)
 }
-
-/**
- * Calls `abortController.abort(reason)` and patches `signal.reason`.
- * if it is not supported.
- *
- * At the time of writing `signal.reason` is available in FF chrome, edge node 17 and deno.
- * @param abortController
- * @param reason
- * @returns
- * @see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason
- */
-export const abortControllerWithReason = <T>(
-  abortController: AbortController,
-  reason: T,
-): void => {
-  type Consumer<T> = (val: T) => void
-
-  const signal = abortController.signal as AbortSignalWithReason<T>
-
-  if (signal.aborted) {
-    return
-  }
-
-  // Patch `reason` if necessary.
-  // - We use defineProperty here because reason is a getter of `AbortSignal.__proto__`.
-  // - We need to patch 'reason' before calling `.abort()` because listeners to the 'abort'
-  // event are are notified immediately.
-  if (!('reason' in signal)) {
-    Object.defineProperty(signal, 'reason', {
-      enumerable: true,
-      value: reason,
-      configurable: true,
-      writable: true,
-    })
-  }
-
-  ;(abortController.abort as Consumer<typeof reason>)(reason)
-}

packages/toolkit/src/query/fetchBaseQuery.ts

@@ -2,6 +2,7 @@ import { joinUrls } from './utils'
 import { isPlainObject } from './core/rtkImports'
 import type { BaseQueryApi, BaseQueryFn } from './baseQueryTypes'
 import type { MaybePromise, Override } from './tsHelpers'
+import { anySignal, timeoutSignal } from './utils/signals'
 
 export type ResponseHandler =
   | 'content-type'
@@ -233,17 +234,11 @@ export function fetchBaseQuery({
       ...rest
     } = typeof arg == 'string' ? { url: arg } : arg
 
-    let abortController: AbortController | undefined,
-      signal = api.signal
-    if (timeout) {
-      abortController = new AbortController()
-      api.signal.addEventListener('abort', abortController.abort)
-      signal = abortController.signal
-    }
-
     let config: RequestInit = {
       ...baseFetchOptions,
-      signal,
+      signal: timeout
+        ? anySignal(api.signal, timeoutSignal(timeout))
+        : api.signal,
       ...rest,
     }
 
@@ -303,30 +298,20 @@ export function fetchBaseQuery({
     const requestClone = new Request(url, config)
     meta = { request: requestClone }
 
-    let response,
-      timedOut = false,
-      timeoutId =
-        abortController &&
-        setTimeout(() => {
-          timedOut = true
-          abortController!.abort()
-        }, timeout)
+    let response
     try {
       response = await fetchFn(request)
     } catch (e) {
       return {
         error: {
-          status: timedOut ? 'TIMEOUT_ERROR' : 'FETCH_ERROR',
+          status:
+            e instanceof DOMException && e.name === 'TimeoutError'
+              ? 'TIMEOUT_ERROR'
+              : 'FETCH_ERROR',
           error: String(e),
         },
         meta,
       }
-    } finally {
-      if (timeoutId) clearTimeout(timeoutId)
-      abortController?.signal.removeEventListener(
-        'abort',
-        abortController.abort,
-      )
     }
     const responseClone = response.clone()
 

packages/toolkit/src/query/tests/createApi.test.ts

@@ -1197,7 +1197,7 @@ describe('timeout behavior', () => {
 
     expect(result?.error).toEqual({
       status: 'TIMEOUT_ERROR',
-      error: expect.stringMatching(/^AbortError:/),
+      error: expect.stringMatching(/^TimeoutError/),
     })
   })
 })