Skip to content

Commit 55036a4

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Fix hook and tests
1 parent cb363db commit 55036a4

File tree

2 files changed

+37
-74
lines changed

2 files changed

+37
-74
lines changed

‎site/src/hooks/useWithRetry.test.ts

Lines changed: 10 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ describe("useWithRetry", () => {
2020
const { result } = renderHook(() => useWithRetry(mockFn));
2121

2222
expect(result.current.isLoading).toBe(false);
23-
expect(result.current.retryAt).toBe(null);
24-
expect(result.current.attemptCount).toBe(0);
23+
expect(result.current.retryAt).toBe(undefined);
2524
});
2625

2726
it("should execute function successfully on first attempt", async () => {
@@ -35,8 +34,7 @@ describe("useWithRetry", () => {
3534

3635
expect(mockFn).toHaveBeenCalledTimes(1);
3736
expect(result.current.isLoading).toBe(false);
38-
expect(result.current.retryAt).toBe(null);
39-
expect(result.current.attemptCount).toBe(0);
37+
expect(result.current.retryAt).toBe(undefined);
4038
});
4139

4240
it("should set isLoading to true during execution", async () => {
@@ -95,7 +93,7 @@ describe("useWithRetry", () => {
9593

9694
expect(mockFn).toHaveBeenCalledTimes(3);
9795
expect(result.current.isLoading).toBe(false);
98-
expect(result.current.retryAt).toBe(null);
96+
expect(result.current.retryAt).toBe(undefined);
9997
});
10098

10199
it("should stop retrying after max attempts (10)", async () => {
@@ -123,8 +121,7 @@ describe("useWithRetry", () => {
123121

124122
// After 10 attempts, should stop retrying
125123
expect(result.current.isLoading).toBe(false);
126-
expect(result.current.retryAt).toBe(null);
127-
expect(result.current.attemptCount).toBe(10); // Should preserve final attempt count
124+
expect(result.current.retryAt).toBe(undefined);
128125
});
129126

130127
it("should respect max delay of 10 minutes", async () => {
@@ -161,7 +158,7 @@ describe("useWithRetry", () => {
161158

162159
expect(mockFn).toHaveBeenCalledTimes(10);
163160
expect(result.current.isLoading).toBe(false);
164-
expect(result.current.retryAt).toBe(null);
161+
expect(result.current.retryAt).toBe(undefined);
165162
});
166163

167164
it("should cancel previous retry when call is invoked again", async () => {
@@ -187,7 +184,7 @@ describe("useWithRetry", () => {
187184

188185
expect(mockFn).toHaveBeenCalledTimes(2);
189186
expect(result.current.isLoading).toBe(false);
190-
expect(result.current.retryAt).toBe(null);
187+
expect(result.current.retryAt).toBe(undefined);
191188

192189
// Advance time to ensure previous retry was cancelled
193190
await act(async () => {
@@ -198,7 +195,9 @@ describe("useWithRetry", () => {
198195
});
199196

200197
it("should set retryAt when scheduling retry", async () => {
201-
mockFn.mockRejectedValue(new Error("Failure"));
198+
mockFn
199+
.mockRejectedValueOnce(new Error("Failure"))
200+
.mockResolvedValueOnce(undefined);
202201

203202
const { result } = renderHook(() => useWithRetry(mockFn));
204203

@@ -221,7 +220,7 @@ describe("useWithRetry", () => {
221220
jest.advanceTimersByTime(1000);
222221
});
223222

224-
expect(result.current.retryAt).toBe(null);
223+
expect(result.current.retryAt).toBe(undefined);
225224
});
226225

227226
it("should cleanup timer on unmount", async () => {
@@ -248,38 +247,4 @@ describe("useWithRetry", () => {
248247
// Function should not have been called again
249248
expect(mockFn).toHaveBeenCalledTimes(1);
250249
});
251-
252-
it("should preserve attemptCount when max attempts reached", async () => {
253-
mockFn.mockRejectedValue(new Error("Always fails"));
254-
255-
const { result } = renderHook(() => useWithRetry(mockFn));
256-
257-
// Start the call
258-
await act(async () => {
259-
await result.current.call();
260-
});
261-
262-
expect(result.current.attemptCount).toBe(1);
263-
264-
// Fast-forward through 9 more retries to reach max attempts
265-
for (let i = 1; i < 10; i++) {
266-
const delay = Math.min(1000 * 2 ** (i - 1), 600000);
267-
await act(async () => {
268-
jest.advanceTimersByTime(delay);
269-
});
270-
expect(result.current.attemptCount).toBe(i + 1);
271-
}
272-
273-
// After max attempts, attemptCount should be preserved
274-
expect(result.current.attemptCount).toBe(10);
275-
expect(result.current.isLoading).toBe(false);
276-
expect(result.current.retryAt).toBe(null);
277-
278-
// Calling again should reset attemptCount
279-
await act(async () => {
280-
await result.current.call();
281-
});
282-
283-
expect(result.current.attemptCount).toBe(1); // Reset on new call
284-
});
285250
});

‎site/src/hooks/useWithRetry.ts

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
11
import { useCallback, useEffect, useRef, useState } from "react";
22

3-
// Configuration constants
43
const MAX_ATTEMPTS = 10;
5-
const INITIAL_DELAY = 1000; // 1 second
6-
const MAX_DELAY = 600000; // 10 minutes
4+
const DELAY_MS = 1_000;
5+
const MAX_DELAY_MS = 600_000; // 10 minutes
6+
// Determines how much the delay between retry attempts increases after each
7+
// failure.
78
const MULTIPLIER = 2;
89

910
interface UseWithRetryResult {
1011
call: () => Promise<void>;
11-
retryAt: Date | null;
12+
retryAt: Date | undefined;
1213
isLoading: boolean;
13-
attemptCount: number;
1414
}
1515

1616
interface RetryState {
1717
isLoading: boolean;
18-
retryAt: Date | null;
18+
retryAt: Date | undefined;
1919
attemptCount: number;
2020
}
2121

@@ -26,71 +26,69 @@ interface RetryState {
2626
export function useWithRetry(fn: () => Promise<void>): UseWithRetryResult {
2727
const [state, setState] = useState<RetryState>({
2828
isLoading: false,
29-
retryAt: null,
29+
retryAt: undefined,
3030
attemptCount: 0,
3131
});
3232

3333
const timeoutRef = useRef<number | null>(null);
3434

35-
const clearTimer = useCallback(() => {
35+
const clearTimeout = useCallback(() => {
3636
if (timeoutRef.current) {
3737
window.clearTimeout(timeoutRef.current);
3838
timeoutRef.current = null;
3939
}
4040
}, []);
4141

4242
const call = useCallback(async () => {
43-
// Cancel any existing retry and start fresh
44-
clearTimer();
43+
clearTimeout();
4544

4645
const executeAttempt = async (attempt: number): Promise<void> => {
4746
setState((prev) => ({ ...prev, isLoading: true, attemptCount: attempt }));
4847

4948
try {
5049
await fn();
51-
// Success - reset everything
52-
setState({ isLoading: false, retryAt: null, attemptCount: 0 });
50+
setState({ isLoading: false, retryAt: undefined, attemptCount: 0 });
5351
} catch (error) {
54-
// Failure - schedule retry if attempts remaining
55-
if (attempt < MAX_ATTEMPTS) {
56-
const delay = Math.min(
57-
INITIAL_DELAY * MULTIPLIER ** attempt,
58-
MAX_DELAY,
52+
// Since attempts start from 0, we need to add +1 to make the condition work
53+
// This ensures exactly MAX_ATTEMPTS total attempts (attempt 0, 1, 2, ..., 9)
54+
if (attempt + 1 < MAX_ATTEMPTS) {
55+
const delayMs = Math.min(
56+
DELAY_MS * MULTIPLIER ** attempt,
57+
MAX_DELAY_MS,
5958
);
60-
const retryTime = new Date(Date.now() + delay);
6159

6260
setState((prev) => ({
6361
...prev,
6462
isLoading: false,
65-
retryAt: retryTime,
63+
retryAt: new Date(Date.now() + delayMs),
6664
}));
6765

68-
// Schedule the actual retry
6966
timeoutRef.current = window.setTimeout(() => {
70-
setState((prev) => ({ ...prev, retryAt: null }));
67+
setState((prev) => ({ ...prev, retryAt: undefined }));
7168
executeAttempt(attempt + 1);
72-
}, delay);
69+
}, delayMs);
7370
} else {
74-
// No more attempts - keep attemptCount for tracking
75-
setState((prev) => ({ ...prev, isLoading: false, retryAt: null }));
71+
setState((prev) => ({
72+
...prev,
73+
isLoading: false,
74+
retryAt: undefined,
75+
}));
7676
}
7777
}
7878
};
7979

8080
await executeAttempt(0);
81-
}, [fn, clearTimer]);
81+
}, [fn, clearTimeout]);
8282

83-
// Cleanup on unmount
8483
useEffect(() => {
8584
return () => {
86-
clearTimer();
85+
clearTimeout();
8786
};
88-
}, [clearTimer]);
87+
}, [clearTimeout]);
8988

9089
return {
9190
call,
9291
retryAt: state.retryAt,
9392
isLoading: state.isLoading,
94-
attemptCount: state.attemptCount,
9593
};
9694
}

0 commit comments

Comments
 (0)