Make maxRetries and retryCondition mutually exclusive

Author: markerikson

packages/toolkit/src/query/retry.ts

@@ -1,4 +1,10 @@
-import type { BaseQueryApi, BaseQueryArg, BaseQueryEnhancer, BaseQueryExtraOptions, BaseQueryFn } from './baseQueryTypes'
+import type {
+  BaseQueryApi,
+  BaseQueryArg,
+  BaseQueryEnhancer,
+  BaseQueryExtraOptions,
+  BaseQueryFn,
+} from './baseQueryTypes'
 import { FetchBaseQueryError } from './fetchBaseQuery'
 import { HandledError } from './HandledError'
 
@@ -24,42 +30,71 @@ async function defaultBackoff(attempt: number = 0, maxRetries: number = 5) {
   )
 }
 
-export interface RetryOptions {
-  /**
-   * How many times the query will be retried (default: 5)
-   */
-  maxRetries?: number
+type RetryConditionFunction = (
+  error: FetchBaseQueryError,
+  args: BaseQueryArg<BaseQueryFn>,
+  extraArgs: {
+    attempt: number
+    baseQueryApi: BaseQueryApi
+    extraOptions: BaseQueryExtraOptions<BaseQueryFn> & RetryOptions
+  }
+) => boolean
+
+export type RetryOptions = {
   /**
    * Function used to determine delay between retries
    */
   backoff?: (attempt: number, maxRetries: number) => Promise<void>
-  /**
-   * Callback to determine if a retry should be attempted.
-   * Return `true` for another retry and `false` to quit trying prematurely.
-   */
-  retryCondition?: (error: FetchBaseQueryError, args: BaseQueryArg<BaseQueryFn>, extraArgs: {
-    attempt: number
-    maxRetries: number
-    baseQueryApi: BaseQueryApi
-    extraOptions: BaseQueryExtraOptions<BaseQueryFn> & RetryOptions
-  }) => boolean
-}
+} & (
+  | {
+      /**
+       * How many times the query will be retried (default: 5)
+       */
+      maxRetries?: number
+      retryCondition?: undefined
+    }
+  | {
+      /**
+       * Callback to determine if a retry should be attempted.
+       * Return `true` for another retry and `false` to quit trying prematurely.
+       */
+      retryCondition?: RetryConditionFunction
+      maxRetries?: undefined
+    }
+)
 
 function fail(e: any): never {
   throw Object.assign(new HandledError({ error: e }), {
     throwImmediately: true,
   })
 }
 
+const EMPTY_OPTIONS = {}
+
 const retryWithBackoff: BaseQueryEnhancer<
   unknown,
   RetryOptions,
   RetryOptions | void
 > = (baseQuery, defaultOptions) => async (args, api, extraOptions) => {
-  const defaultRetryCondition: Exclude<RetryOptions['retryCondition'], undefined> = (_, __, {attempt, maxRetries}) => attempt <= maxRetries
+  // We need to figure out `maxRetries` before we define `defaultRetryCondition.
+  // This is probably goofy, but ought to work.
+  // Put our defaults in one array, filter out undefineds, grab the last value.
+  const possibleMaxRetries: number[] = [
+    5,
+    ((defaultOptions as any) || EMPTY_OPTIONS).maxRetries,
+    ((extraOptions as any) || EMPTY_OPTIONS).maxRetries,
+  ].filter(Boolean)
+  const [maxRetries] = possibleMaxRetries.slice(-1)
 
-  const options = {
-    maxRetries: 5,
+  const defaultRetryCondition: RetryConditionFunction = (_, __, { attempt }) =>
+    attempt <= maxRetries
+
+  const options: {
+    maxRetries: number
+    backoff: typeof defaultBackoff
+    retryCondition: typeof defaultRetryCondition
+  } = {
+    maxRetries,
     backoff: defaultBackoff,
     retryCondition: defaultRetryCondition,
     ...defaultOptions,
@@ -87,12 +122,14 @@ const retryWithBackoff: BaseQueryEnhancer<
         throw e
       }
 
-      if (e instanceof HandledError && !options.retryCondition(e.value as FetchBaseQueryError, args, {
-        attempt: retry,
-        maxRetries: options.maxRetries,
-        baseQueryApi: api,
-        extraOptions
-      })) {
+      if (
+        e instanceof HandledError &&
+        !options.retryCondition(e.value as FetchBaseQueryError, args, {
+          attempt: retry,
+          baseQueryApi: api,
+          extraOptions,
+        })
+      ) {
         return e.value
       }
       await options.backoff(retry, options.maxRetries)

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

@@ -1,6 +1,7 @@
 import type { BaseQueryFn } from '@reduxjs/toolkit/query'
 import { createApi, retry } from '@reduxjs/toolkit/query'
 import { setupApiStore, waitMs } from './helpers'
+import type { RetryOptions } from '../retry'
 
 beforeEach(() => {
   jest.useFakeTimers('legacy')
@@ -350,8 +351,7 @@ describe('configuration', () => {
     const overrideMaxRetries = 3
 
     const baseQuery = retry(baseBaseQuery, {
-      maxRetries: 0,
-      retryCondition: (_, __, {attempt}) => attempt <= overrideMaxRetries,
+      retryCondition: (_, __, { attempt }) => attempt <= overrideMaxRetries,
     })
     const api = createApi({
       baseQuery,
@@ -380,16 +380,16 @@ describe('configuration', () => {
     baseBaseQuery.mockResolvedValue({ error: 'rejected' })
 
     const baseQuery = retry(baseBaseQuery, {
-      maxRetries: 10
+      maxRetries: 10,
     })
     const api = createApi({
       baseQuery,
       endpoints: (build) => ({
         q1: build.query({
           query: () => {},
           extraOptions: {
-            retryCondition: (_, __, {attempt, maxRetries}) => attempt <= maxRetries / 2,
-          }
+            retryCondition: (_, __, { attempt }) => attempt <= 5,
+          },
         }),
       }),
     })
@@ -424,7 +424,7 @@ describe('configuration', () => {
           query: () => ({ method: 'PUT' }),
           extraOptions: {
             retryCondition: (e) => e.data === 'hello retryCondition',
-          }
+          },
         }),
       }),
     })
@@ -438,4 +438,12 @@ describe('configuration', () => {
 
     expect(baseBaseQuery).toHaveBeenCalledTimes(4)
   })
+
+  test.skip('RetryOptions only accepts one of maxRetries or retryCondition', () => {
+    // @ts-expect-error Should complain if both exist at once
+    const ro: RetryOptions = {
+      maxRetries: 5,
+      retryCondition: () => false,
+    }
+  })
 })