From 1fabe3586cd74c2a396a2ca8086c60a6b33da614 Mon Sep 17 00:00:00 2001
From: Dois <DoisKoh@users.noreply.github.com>
Date: Thu, 18 Jan 2024 02:05:15 +0800
Subject: [PATCH] fix: retry delay not persisting between calls (#3478)

Co-authored-by: Jovi De Croock <decroockjovi@gmail.com>
---
 .changeset/witty-peas-love.md             |  7 +++
 exchanges/retry/src/retryExchange.test.ts | 64 +++++++++++++++++++++--
 exchanges/retry/src/retryExchange.ts      | 21 ++++++--
 3 files changed, 84 insertions(+), 8 deletions(-)
 create mode 100644 .changeset/witty-peas-love.md

diff --git a/.changeset/witty-peas-love.md b/.changeset/witty-peas-love.md
new file mode 100644
index 0000000000..850dad9de2
--- /dev/null
+++ b/.changeset/witty-peas-love.md
@@ -0,0 +1,7 @@
+---
+'@urql/exchange-retry': patch
+---
+
+---
+
+Fixed the delay amount not increasing as retry count increases.
diff --git a/exchanges/retry/src/retryExchange.test.ts b/exchanges/retry/src/retryExchange.test.ts
index 13de90f8cc..a56ee8c4d4 100644
--- a/exchanges/retry/src/retryExchange.test.ts
+++ b/exchanges/retry/src/retryExchange.test.ts
@@ -211,15 +211,15 @@ it('should reset the retry counter if an operation succeeded first', () => {
       return fromArray([
         {
           operation: forwardOp,
-          data: queryOneData,
+          error: queryOneError,
         } as any,
         {
           operation: forwardOp,
-          error: queryOneError,
+          data: queryOneData,
         } as any,
       ]);
     } else {
-      expect(forwardOp.context.retry).toEqual({ count: 1, delay: null });
+      expect(forwardOp.context.retry).toEqual({ count: 0, delay: null });
 
       return fromValue({
         operation: forwardOp,
@@ -396,3 +396,61 @@ it('should allow retryWhen to return new operations when retrying', () => {
   expect(response.mock.calls[1][0]).toHaveProperty('context.counter', 1);
   expect(response.mock.calls[2][0]).toHaveProperty('context.counter', 2);
 });
+
+it('should increase retries by initialDelayMs for each subsequent failure', () => {
+  const errorWithNetworkError = {
+    ...queryOneError,
+    networkError: 'scary network error',
+  };
+  const response = vi.fn((forwardOp: Operation): OperationResult => {
+    expect(forwardOp.key).toBe(op.key);
+    return {
+      operation: forwardOp,
+      // @ts-ignore
+      error: errorWithNetworkError,
+    };
+  });
+
+  const result = vi.fn();
+  const forward: ExchangeIO = ops$ => {
+    return pipe(ops$, map(response));
+  };
+
+  const retryWith = vi.fn((_error, operation) => {
+    return makeOperation(operation.kind, operation, {
+      ...operation.context,
+      counter: (operation.context?.counter || 0) + 1,
+    });
+  });
+
+  const fixedDelayMs = 50;
+
+  const fixedDelayOptions = {
+    ...mockOptions,
+    randomDelay: false,
+    initialDelayMs: fixedDelayMs,
+  };
+
+  pipe(
+    retryExchange({
+      ...fixedDelayOptions,
+      retryIf: undefined,
+      retryWith,
+    })({
+      forward,
+      client,
+      dispatchDebug,
+    })(ops$),
+    tap(result),
+    publish
+  );
+
+  next(op);
+
+  // delay between each call should be increased by initialDelayMs
+  // (e.g. if initialDelayMs is 5s, first retry is waits 5 seconds, second retry waits 10 seconds)
+  for (let i = 1; i <= fixedDelayOptions.maxNumberAttempts; i++) {
+    expect(response).toHaveBeenCalledTimes(i);
+    vi.advanceTimersByTime(i * fixedDelayOptions.initialDelayMs);
+  }
+});
diff --git a/exchanges/retry/src/retryExchange.ts b/exchanges/retry/src/retryExchange.ts
index 98bd020f00..2410fc4657 100644
--- a/exchanges/retry/src/retryExchange.ts
+++ b/exchanges/retry/src/retryExchange.ts
@@ -111,7 +111,7 @@ interface RetryState {
 export const retryExchange = (options: RetryExchangeOptions): Exchange => {
   const { retryIf, retryWith } = options;
   const MIN_DELAY = options.initialDelayMs || 1000;
-  const MAX_DELAY = options.maxDelayMs || 15000;
+  const MAX_DELAY = options.maxDelayMs || 15_000;
   const MAX_ATTEMPTS = options.maxNumberAttempts || 2;
   const RANDOM_DELAY =
     options.randomDelay != null ? !!options.randomDelay : true;
@@ -133,12 +133,22 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
           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
-          // amount to the delay to avoid thundering herd problem
-          if (RANDOM_DELAY && delayAmount * backoffFactor < MAX_DELAY) {
-            delayAmount *= backoffFactor;
+          if (RANDOM_DELAY) {
+            // if randomDelay is enabled and it won't exceed the max delay, apply a random
+            // amount to the delay to avoid thundering herd problem
+            if (delayAmount * backoffFactor < MAX_DELAY) {
+              delayAmount *= backoffFactor;
+            } else {
+              delayAmount = MAX_DELAY;
+            }
+          } else {
+            // otherwise, increase the delay proportionately by the initial delay
+            delayAmount = Math.min(retryCount * MIN_DELAY, MAX_DELAY);
           }
 
+          // ensure the delay is carried over to the next context
+          retry.delay = delayAmount;
+
           // We stop the retries if a teardown event for this operation comes in
           // But if this event comes through regularly we also stop the retries, since it's
           // basically the query retrying itself, no backoff should be added!
@@ -158,6 +168,7 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
             operation,
             data: {
               retryCount,
+              delayAmount,
             },
           });