Skip to content

Commit

Permalink
fix: retry delay not persisting between calls (#3478)
Browse files Browse the repository at this point in the history
Co-authored-by: Jovi De Croock <[email protected]>
  • Loading branch information
DoisKoh and JoviDeCroock authored Jan 17, 2024
1 parent 93d7256 commit 1fabe35
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 8 deletions.
7 changes: 7 additions & 0 deletions .changeset/witty-peas-love.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@urql/exchange-retry': patch
---

---

Fixed the delay amount not increasing as retry count increases.
64 changes: 61 additions & 3 deletions exchanges/retry/src/retryExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
});
21 changes: 16 additions & 5 deletions exchanges/retry/src/retryExchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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!
Expand All @@ -158,6 +168,7 @@ export const retryExchange = (options: RetryExchangeOptions): Exchange => {
operation,
data: {
retryCount,
delayAmount,
},
});

Expand Down

0 comments on commit 1fabe35

Please sign in to comment.