fix(retry): Reset delay/count state if an operation succeeds (#3229)

.changeset/curvy-poems-compare.md

@@ -0,0 +1,5 @@
+---
+'@urql/exchange-retry': minor
+---
+
+Reset `retryExchange`’s previous attempts and delay if an operation succeeds. This prevents the exchange from keeping its old retry count and delay if the operation delivered a result in the meantime. This is important for it to help recover from failing subscriptions.

exchanges/retry/src/retryExchange.test.ts

@@ -1,4 +1,14 @@
-import { pipe, map, makeSubject, publish, tap } from 'wonka';
+import {
+  Source,
+  pipe,
+  map,
+  makeSubject,
+  mergeMap,
+  fromValue,
+  fromArray,
+  publish,
+  tap,
+} from 'wonka';
 import { vi, expect, it, beforeEach, afterEach } from 'vitest';
 
 import {
@@ -67,7 +77,7 @@ beforeEach(() => {
   ({ source: ops$, next } = makeSubject<Operation>());
 });
 
-it(`retries if it hits an error and works for multiple concurrent operations`, () => {
+it('retries if it hits an error and works for multiple concurrent operations', () => {
   const queryTwo = gql`
     {
       films {
@@ -149,7 +159,7 @@ it('should retry x number of times and then return the successful result', () =>
     // @ts-ignore
     return {
       operation: forwardOp,
-      ...(forwardOp.context.retryCount! >= numberRetriesBeforeSuccess
+      ...(forwardOp.context.retry?.count >= numberRetriesBeforeSuccess
         ? { data: queryOneData }
         : { error: queryOneError }),
     };
@@ -186,6 +196,68 @@ it('should retry x number of times and then return the successful result', () =>
   expect(result).toHaveBeenCalledTimes(1);
 });
 
+it('should reset the retry counter if an operation succeeded first', () => {
+  let call = 0;
+  const response = vi.fn((forwardOp: Operation): Source<any> => {
+    expect(forwardOp.key).toBe(op.key);
+    if (call === 0) {
+      call++;
+      return fromValue({
+        operation: forwardOp,
+        error: queryOneError,
+      } as any);
+    } else if (call === 1) {
+      call++;
+      return fromArray([
+        {
+          operation: forwardOp,
+          data: queryOneData,
+        } as any,
+        {
+          operation: forwardOp,
+          error: queryOneError,
+        } as any,
+      ]);
+    } else {
+      expect(forwardOp.context.retry).toEqual({ count: 1, delay: null });
+
+      return fromValue({
+        operation: forwardOp,
+        data: queryOneData,
+      } as any);
+    }
+  });
+
+  const result = vi.fn();
+  const forward: ExchangeIO = ops$ => {
+    return pipe(ops$, mergeMap(response));
+  };
+
+  const mockRetryIf = vi.fn(() => true);
+
+  pipe(
+    retryExchange({
+      ...mockOptions,
+      retryIf: mockRetryIf,
+    })({
+      forward,
+      client,
+      dispatchDebug,
+    })(ops$),
+    tap(result),
+    publish
+  );
+
+  next(op);
+  vi.runAllTimers();
+
+  expect(mockRetryIf).toHaveBeenCalledTimes(2);
+  expect(mockRetryIf).toHaveBeenCalledWith(queryOneError as any, op);
+
+  expect(response).toHaveBeenCalledTimes(3);
+  expect(result).toHaveBeenCalledTimes(2);
+});
+
 it(`should still retry if retryIf undefined but there is a networkError`, () => {
   const errorWithNetworkError = {
     ...queryOneError,

exchanges/retry/src/retryExchange.ts

@@ -78,6 +78,11 @@ export interface RetryExchangeOptions {
   ): Operation | null | undefined;
 }
 
+interface RetryState {
+  count: number;
+  delay: number | null;
+}
+
 /** Exchange factory that retries failed operations.
  *
  * @param options - A {@link RetriesExchangeOptions} configuration object.
@@ -117,10 +122,14 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
 
       const retryWithBackoff$ = pipe(
         retry$,
-        mergeMap((op: Operation) => {
-          const { key, context } = op;
-          const retryCount = (context.retryCount || 0) + 1;
-          let delayAmount = context.retryDelay || MIN_DELAY;
+        mergeMap((operation: Operation) => {
+          const retry: RetryState = operation.context.retry || {
+            count: 0,
+            delay: null,
+          };
+
+          const retryCount = ++retry.count;
+          let delayAmount = retry.delay || MIN_DELAY;
 
           const backoffFactor = Math.random() + 1.5;
           // if randomDelay is enabled and it won't exceed the max delay, apply a random
@@ -137,15 +146,15 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
             filter(op => {
               return (
                 (op.kind === 'query' || op.kind === 'teardown') &&
-                op.key === key
+                op.key === operation.key
               );
             })
           );
 
           dispatchDebug({
             type: 'retryAttempt',
             message: `The operation has failed and a retry has been triggered (${retryCount} / ${MAX_ATTEMPTS})`,
-            operation: op,
+            operation,
             data: {
               retryCount,
             },
@@ -154,10 +163,9 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
           // Add new retryDelay and retryCount to operation
           return pipe(
             fromValue(
-              makeOperation(op.kind, op, {
-                ...op.context,
-                retryDelay: delayAmount,
-                retryCount,
+              makeOperation(operation.kind, operation, {
+                ...operation.context,
+                retry,
               })
             ),
             debounce(() => delayAmount),
@@ -171,6 +179,7 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
         merge([operations$, retryWithBackoff$]),
         forward,
         filter(res => {
+          const retry = res.operation.context.retry as RetryState | undefined;
           // Only retry if the error passes the conditional retryIf function (if passed)
           // or if the error contains a networkError
           if (
@@ -179,12 +188,16 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
               ? !retryIf(res.error, res.operation)
               : !retryWith && !res.error.networkError)
           ) {
+            // Reset the delay state for a successful operation
+            if (retry) {
+              retry.count = 0;
+              retry.delay = null;
+            }
             return true;
           }
 
           const maxNumberAttemptsExceeded =
-            (res.operation.context.retryCount || 0) >= MAX_ATTEMPTS - 1;
-
+            ((retry && retry.count) || 0) >= MAX_ATTEMPTS - 1;
           if (!maxNumberAttemptsExceeded) {
             const operation = retryWith
               ? retryWith(res.error, res.operation)