Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: retry delay not persisting between calls #3478

Merged
merged 2 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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