Skip to content

Commit 3a1c531

Browse files
viva-jinyiclaude
andcommitted
fix: retry on API errors in session cookie creation
- Continue retry loop when API returns non-ok response instead of throwing immediately - Ensures 401/403 errors trigger token refresh retry as intended - Only throw error on final attempt to preserve error details - Add comprehensive unit tests for retry scenarios including token timing issues Addresses Claude Code review feedback on retry logic error handling 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 228f7fa commit 3a1c531

File tree

2 files changed

+282
-3
lines changed

2 files changed

+282
-3
lines changed
Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
3+
import { useSessionCookie } from './useSessionCookie'
4+
5+
// Mock fetch
6+
const mockFetch = vi.fn()
7+
vi.stubGlobal('fetch', mockFetch)
8+
9+
// Mock api.apiURL
10+
vi.mock('@/scripts/api', () => ({
11+
api: {
12+
apiURL: vi.fn((path: string) => `https://test-api.com${path}`)
13+
}
14+
}))
15+
16+
// Mock isCloud to always return true for testing
17+
vi.mock('@/platform/distribution/types', () => ({
18+
isCloud: true
19+
}))
20+
21+
// Mock useFirebaseAuthStore
22+
const mockGetAuthHeader = vi.fn()
23+
vi.mock('@/stores/firebaseAuthStore', () => ({
24+
useFirebaseAuthStore: vi.fn(() => ({
25+
getAuthHeader: mockGetAuthHeader
26+
}))
27+
}))
28+
29+
describe('useSessionCookie', () => {
30+
beforeEach(() => {
31+
vi.resetAllMocks()
32+
mockFetch.mockReset()
33+
mockGetAuthHeader.mockReset()
34+
})
35+
36+
describe('createSession', () => {
37+
it('should succeed on first attempt', async () => {
38+
// Mock successful auth header and API response
39+
mockGetAuthHeader.mockResolvedValue({ Authorization: 'Bearer token' })
40+
mockFetch.mockResolvedValue({
41+
ok: true,
42+
status: 200
43+
})
44+
45+
const { createSession } = useSessionCookie()
46+
await createSession()
47+
48+
expect(mockGetAuthHeader).toHaveBeenCalledWith(false) // First attempt with cached token
49+
expect(mockFetch).toHaveBeenCalledWith(
50+
'https://test-api.com/auth/session',
51+
{
52+
method: 'POST',
53+
credentials: 'include',
54+
headers: {
55+
Authorization: 'Bearer token',
56+
'Content-Type': 'application/json'
57+
}
58+
}
59+
)
60+
})
61+
62+
it('should retry with fresh token after 401 error and succeed', async () => {
63+
// First attempt: auth header success but API returns 401
64+
// Second attempt: fresh token and API success
65+
mockGetAuthHeader
66+
.mockResolvedValueOnce({ Authorization: 'Bearer cached-token' })
67+
.mockResolvedValueOnce({ Authorization: 'Bearer fresh-token' })
68+
69+
mockFetch
70+
.mockResolvedValueOnce({
71+
ok: false,
72+
status: 401,
73+
statusText: 'Unauthorized',
74+
json: () => Promise.resolve({ message: 'Token expired' })
75+
})
76+
.mockResolvedValueOnce({
77+
ok: true,
78+
status: 200
79+
})
80+
81+
const { createSession } = useSessionCookie()
82+
await createSession()
83+
84+
expect(mockGetAuthHeader).toHaveBeenCalledTimes(2)
85+
expect(mockGetAuthHeader).toHaveBeenNthCalledWith(1, false) // First attempt with cached token
86+
expect(mockGetAuthHeader).toHaveBeenNthCalledWith(2, true) // Second attempt with forced refresh
87+
88+
expect(mockFetch).toHaveBeenCalledTimes(2)
89+
expect(mockFetch).toHaveBeenNthCalledWith(
90+
1,
91+
'https://test-api.com/auth/session',
92+
{
93+
method: 'POST',
94+
credentials: 'include',
95+
headers: {
96+
Authorization: 'Bearer cached-token',
97+
'Content-Type': 'application/json'
98+
}
99+
}
100+
)
101+
expect(mockFetch).toHaveBeenNthCalledWith(
102+
2,
103+
'https://test-api.com/auth/session',
104+
{
105+
method: 'POST',
106+
credentials: 'include',
107+
headers: {
108+
Authorization: 'Bearer fresh-token',
109+
'Content-Type': 'application/json'
110+
}
111+
}
112+
)
113+
})
114+
115+
it('should fail after all retries with API error', async () => {
116+
// All attempts return auth headers but API always fails
117+
mockGetAuthHeader.mockResolvedValue({ Authorization: 'Bearer token' })
118+
mockFetch.mockResolvedValue({
119+
ok: false,
120+
status: 500,
121+
statusText: 'Internal Server Error',
122+
json: () => Promise.resolve({ message: 'Server error' })
123+
})
124+
125+
const { createSession } = useSessionCookie()
126+
127+
await expect(createSession()).rejects.toThrow(
128+
'Failed to create session: Server error'
129+
)
130+
131+
expect(mockGetAuthHeader).toHaveBeenCalledTimes(3)
132+
expect(mockFetch).toHaveBeenCalledTimes(3)
133+
})
134+
135+
it('should succeed after auth header null then fresh token success', async () => {
136+
// First attempt: no auth header (token timing issue)
137+
// Second attempt: fresh token success
138+
mockGetAuthHeader
139+
.mockResolvedValueOnce(null)
140+
.mockResolvedValueOnce({ Authorization: 'Bearer fresh-token' })
141+
142+
mockFetch.mockResolvedValue({
143+
ok: true,
144+
status: 200
145+
})
146+
147+
const { createSession } = useSessionCookie()
148+
await createSession()
149+
150+
expect(mockGetAuthHeader).toHaveBeenCalledTimes(2)
151+
expect(mockGetAuthHeader).toHaveBeenNthCalledWith(1, false) // First attempt with cached token
152+
expect(mockGetAuthHeader).toHaveBeenNthCalledWith(2, true) // Second attempt with forced refresh
153+
154+
expect(mockFetch).toHaveBeenCalledTimes(1) // Only called when auth header is available
155+
expect(mockFetch).toHaveBeenCalledWith(
156+
'https://test-api.com/auth/session',
157+
{
158+
method: 'POST',
159+
credentials: 'include',
160+
headers: {
161+
Authorization: 'Bearer fresh-token',
162+
'Content-Type': 'application/json'
163+
}
164+
}
165+
)
166+
})
167+
168+
it('should fail when no auth header is available after all retries', async () => {
169+
// All attempts fail to get auth header (complete auth failure)
170+
mockGetAuthHeader.mockResolvedValue(null)
171+
172+
const { createSession } = useSessionCookie()
173+
174+
await expect(createSession()).rejects.toThrow(
175+
'No auth header available for session creation after retries'
176+
)
177+
178+
expect(mockGetAuthHeader).toHaveBeenCalledTimes(3)
179+
expect(mockFetch).not.toHaveBeenCalled()
180+
})
181+
182+
it('should handle mixed auth header failures and successes', async () => {
183+
// First attempt: no auth header
184+
// Second attempt: auth header but API fails
185+
// Third attempt: auth header and API success
186+
mockGetAuthHeader
187+
.mockResolvedValueOnce(null)
188+
.mockResolvedValueOnce({ Authorization: 'Bearer token' })
189+
.mockResolvedValueOnce({ Authorization: 'Bearer fresh-token' })
190+
191+
mockFetch
192+
.mockResolvedValueOnce({
193+
ok: false,
194+
status: 401,
195+
statusText: 'Unauthorized',
196+
json: () => Promise.resolve({ message: 'Token expired' })
197+
})
198+
.mockResolvedValueOnce({
199+
ok: true,
200+
status: 200
201+
})
202+
203+
const { createSession } = useSessionCookie()
204+
await createSession()
205+
206+
expect(mockGetAuthHeader).toHaveBeenCalledTimes(3)
207+
expect(mockFetch).toHaveBeenCalledTimes(2) // Only called when auth header is available
208+
})
209+
210+
it('should handle JSON parsing errors gracefully', async () => {
211+
mockGetAuthHeader.mockResolvedValue({ Authorization: 'Bearer token' })
212+
mockFetch.mockResolvedValue({
213+
ok: false,
214+
status: 500,
215+
statusText: 'Internal Server Error',
216+
json: () => Promise.reject(new Error('Invalid JSON'))
217+
})
218+
219+
const { createSession } = useSessionCookie()
220+
221+
await expect(createSession()).rejects.toThrow(
222+
'Failed to create session: Internal Server Error'
223+
)
224+
})
225+
})
226+
227+
describe('deleteSession', () => {
228+
it('should successfully delete session', async () => {
229+
mockFetch.mockResolvedValue({
230+
ok: true,
231+
status: 200
232+
})
233+
234+
const { deleteSession } = useSessionCookie()
235+
await deleteSession()
236+
237+
expect(mockFetch).toHaveBeenCalledWith(
238+
'https://test-api.com/auth/session',
239+
{
240+
method: 'DELETE',
241+
credentials: 'include'
242+
}
243+
)
244+
})
245+
246+
it('should handle delete session errors', async () => {
247+
mockFetch.mockResolvedValue({
248+
ok: false,
249+
status: 404,
250+
statusText: 'Not Found',
251+
json: () => Promise.resolve({ message: 'Session not found' })
252+
})
253+
254+
const { deleteSession } = useSessionCookie()
255+
256+
await expect(deleteSession()).rejects.toThrow(
257+
'Failed to delete session: Session not found'
258+
)
259+
})
260+
261+
it('should handle delete session JSON parsing errors', async () => {
262+
mockFetch.mockResolvedValue({
263+
ok: false,
264+
status: 500,
265+
statusText: 'Internal Server Error',
266+
json: () => Promise.reject(new Error('Invalid JSON'))
267+
})
268+
269+
const { deleteSession } = useSessionCookie()
270+
271+
await expect(deleteSession()).rejects.toThrow(
272+
'Failed to delete session: Internal Server Error'
273+
)
274+
})
275+
})
276+
})

src/platform/auth/session/useSessionCookie.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,17 @@ export const useSessionCookie = () => {
3333
}
3434
})
3535

36-
if (!response.ok) {
36+
if (response.ok) {
37+
return // Success
38+
}
39+
40+
// API error - continue retry loop if not the last attempt
41+
if (attempt === 2) {
3742
const errorData = await response.json().catch(() => ({}))
3843
throw new Error(
3944
`Failed to create session: ${errorData.message || response.statusText}`
4045
)
4146
}
42-
43-
return // Success
4447
}
4548

4649
// Exponential backoff before retry (except for last attempt)

0 commit comments

Comments
 (0)