Skip to content

Commit dbdb8e0

Browse files
smarcetsmarcet
committed
fix(refresh-token): improved HTTP error handling
1 parent 31056df commit dbdb8e0

2 files changed

Lines changed: 96 additions & 18 deletions

File tree

‎src/components/security/__tests__/methods.test.js‎

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,36 @@ describe('refreshAccessToken', () => {
157157
.toThrow(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR);
158158
});
159159

160+
it('should throw AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR on HTTP 408 (Request Timeout)', async () => {
161+
const mockResponse = {
162+
ok: false,
163+
status: 408,
164+
statusText: 'Request Timeout',
165+
};
166+
global.fetch = jest.fn().mockResolvedValue(mockResponse);
167+
168+
await expect(refreshAccessToken('valid-token'))
169+
.rejects
170+
.toThrow(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR);
171+
172+
expect(setSessionClearingState).not.toHaveBeenCalled();
173+
});
174+
175+
it('should throw AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR on HTTP 429 (Too Many Requests)', async () => {
176+
const mockResponse = {
177+
ok: false,
178+
status: 429,
179+
statusText: 'Too Many Requests',
180+
};
181+
global.fetch = jest.fn().mockResolvedValue(mockResponse);
182+
183+
await expect(refreshAccessToken('valid-token'))
184+
.rejects
185+
.toThrow(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR);
186+
187+
expect(setSessionClearingState).not.toHaveBeenCalled();
188+
});
189+
160190
it('should throw AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR on network failure', async () => {
161191
global.fetch = jest.fn().mockRejectedValue(new TypeError('Failed to fetch'));
162192

@@ -225,6 +255,32 @@ describe('refreshAccessToken', () => {
225255
.rejects
226256
.toThrow(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: Failed to fetch`);
227257
});
258+
259+
it('should throw retryable error when response body is not valid JSON', async () => {
260+
const mockResponse = {
261+
ok: true,
262+
status: 200,
263+
json: jest.fn().mockRejectedValue(new SyntaxError('Unexpected token < in JSON')),
264+
};
265+
global.fetch = jest.fn().mockResolvedValue(mockResponse);
266+
267+
await expect(refreshAccessToken('valid-token'))
268+
.rejects
269+
.toThrow(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: invalid JSON response from IDP`);
270+
271+
expect(setSessionClearingState).not.toHaveBeenCalled();
272+
});
273+
274+
it('should throw retryable error when fetch is aborted by timeout', async () => {
275+
const abortError = new DOMException('The operation was aborted.', 'AbortError');
276+
global.fetch = jest.fn().mockRejectedValue(abortError);
277+
278+
await expect(refreshAccessToken('valid-token'))
279+
.rejects
280+
.toThrow(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR);
281+
282+
expect(setSessionClearingState).not.toHaveBeenCalled();
283+
});
228284
});
229285

230286
describe('retryWithBackoff', () => {
@@ -241,7 +297,7 @@ describe('retryWithBackoff', () => {
241297

242298
it('should retry network errors up to maxRetries then throw', async () => {
243299
jest.useRealTimers();
244-
const networkError = new Error('network down');
300+
const networkError = new Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: network down`);
245301
const fn = jest.fn().mockRejectedValue(networkError);
246302

247303
await expect(retryWithBackoff(fn, 3, 1))
@@ -254,8 +310,8 @@ describe('retryWithBackoff', () => {
254310
it('should succeed after transient failures', async () => {
255311
jest.useRealTimers();
256312
const fn = jest.fn()
257-
.mockRejectedValueOnce(new Error('transient'))
258-
.mockRejectedValueOnce(new Error('transient'))
313+
.mockRejectedValueOnce(new Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: transient`))
314+
.mockRejectedValueOnce(new Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: transient`))
259315
.mockResolvedValue('recovered');
260316

261317
const result = await retryWithBackoff(fn, 5, 1);
@@ -276,13 +332,25 @@ describe('retryWithBackoff', () => {
276332
expect(fn).toHaveBeenCalledTimes(1);
277333
});
278334

335+
it('should not retry unexpected errors (fail fast)', async () => {
336+
jest.useRealTimers();
337+
const unexpectedError = new TypeError('Cannot read properties of undefined');
338+
const fn = jest.fn().mockRejectedValue(unexpectedError);
339+
340+
await expect(retryWithBackoff(fn, 5, 1))
341+
.rejects
342+
.toThrow('Cannot read properties of undefined');
343+
344+
expect(fn).toHaveBeenCalledTimes(1);
345+
});
346+
279347
it('should apply exponential backoff delays', async () => {
280348
jest.useRealTimers();
281349
const setTimeoutSpy = jest.spyOn(global, 'setTimeout');
282350

283351
const fn = jest.fn()
284-
.mockRejectedValueOnce(new Error('fail 1'))
285-
.mockRejectedValueOnce(new Error('fail 2'))
352+
.mockRejectedValueOnce(new Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: fail 1`))
353+
.mockRejectedValueOnce(new Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: fail 2`))
286354
.mockResolvedValue('ok');
287355

288356
const baseDelay = 100;

‎src/components/security/methods.js‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -266,18 +266,16 @@ export const emitAccessToken = async (code, backUrl = null) => {
266266

267267
export const MAX_RETRIES = 5;
268268
export const BACKOFF_BASE_MS = 1000;
269+
export const REFRESH_TOKEN_FETCH_TIMEOUT_MS = 10000;
269270

270271
export const retryWithBackoff = async (fn, maxRetries = MAX_RETRIES, baseDelayMs = BACKOFF_BASE_MS) => {
271272
for (let attempt = 0; attempt < maxRetries; attempt++) {
272273
try {
273274
return await fn();
274275
} catch (err) {
275-
// auth errors (HTTP 400) are fatal — never retry
276-
if (err.message && err.message.startsWith(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR)) {
277-
throw err;
278-
}
279-
// network/5xx errors are retryable — retry unless exhausted
280-
if (attempt === maxRetries - 1) {
276+
// only retry transient network/server errors — everything else fails fast
277+
const isRetryable = err.message && err.message.startsWith(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR);
278+
if (!isRetryable || attempt === maxRetries - 1) {
281279
throw err;
282280
}
283281
const delay = baseDelayMs * Math.pow(2, attempt);
@@ -339,7 +337,7 @@ const _getAccessToken = async () => {
339337
* @returns {Promise<*|undefined>}
340338
*/
341339
export const getAccessToken = async () => {
342-
if (navigator?.locks) {
340+
if (typeof navigator !== 'undefined' && navigator.locks) {
343341
return await navigator.locks.request(GET_TOKEN_SILENTLY_LOCK_KEY, async lock => {
344342
console.log(`openstack-uicore-foundation::Security::methods::getAccessToken web lock api`, lock);
345343
return await _getAccessToken();
@@ -383,7 +381,7 @@ const _clearAccessToken = () => {
383381

384382
export const clearAccessToken = async () => {
385383
// see https://developer.mozilla.org/en-US/docs/Web/API/Web_Locks_API
386-
if (navigator?.locks) {
384+
if (typeof navigator !== 'undefined' && navigator.locks) {
387385
await navigator.locks.request(GET_TOKEN_SILENTLY_LOCK_KEY, async lock => {
388386
console.log(`openstack-uicore-foundation::Security::methods::clearAccessToken web lock api`, lock);
389387
_clearAccessToken();
@@ -419,6 +417,9 @@ export const refreshAccessToken = async (refresh_token) => {
419417
"refresh_token": refresh_token
420418
};
421419

420+
const controller = new AbortController();
421+
const timeoutId = setTimeout(() => controller.abort(), REFRESH_TOKEN_FETCH_TIMEOUT_MS);
422+
422423
let response;
423424
try {
424425
response = await fetch(`${baseUrl}/oauth2/token`, {
@@ -427,26 +428,35 @@ export const refreshAccessToken = async (refresh_token) => {
427428
'Accept': 'application/json',
428429
'Content-Type': 'application/json'
429430
},
430-
body: JSON.stringify(payload)
431+
body: JSON.stringify(payload),
432+
signal: controller.signal
431433
});
432434
} catch (networkError) {
433-
// fetch rejects on network failures (DNS, timeout, no connectivity)
435+
// fetch rejects on network failures (DNS, timeout, no connectivity, abort)
434436
console.log('refreshAccessToken network error:', networkError.message);
435437
throw Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: ${networkError.message}`);
438+
} finally {
439+
clearTimeout(timeoutId);
436440
}
437441

438442
if (!response.ok) {
439443
console.log(`refreshAccessToken server error: ${response.status} - ${response.statusText}`);
440-
if (response.status >= 500) {
441-
// server error — transient, should be retried
444+
if (response.status >= 500 || response.status === 408 || response.status === 429) {
445+
// transient error (server error, request timeout, rate limit) — should be retried
442446
throw Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: ${response.status} - ${response.statusText}`);
443447
}
444448
// token is genuinely revoked — this is a real auth error
445449
setSessionClearingState(true);
446450
throw Error(`${AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR}: ${response.status} - ${response.statusText}`);
447451
}
448452

449-
const json = await response.json();
453+
let json;
454+
try {
455+
json = await response.json();
456+
} catch (parseError) {
457+
// IDP returned non-JSON (HTML error page, empty body, etc.) — treat as transient
458+
throw Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: invalid JSON response from IDP`);
459+
}
450460
let {access_token, refresh_token: new_refresh_token, expires_in, id_token} = json;
451461
// Defensively ensure we never propagate an undefined access token.
452462
if (!access_token) {

0 commit comments

Comments
 (0)