ix: Fix rece condition when detecting the browser extension.#24
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a race condition in production where API requests could be routed through the proxy fallback before the browser extension detection finished, by making requests wait for a single shared detection promise.
Changes:
- Replace per-call extension detection with a module-level
detectionPromisethat runs once in production. - Make the production Axios request interceptor
asyncand await extension detection before choosing direct vscorsproxy.iorouting.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| const resp = await fetch( | ||
| `${TARGET_BASE}/WCF/LSI/LSIBookingServiceV3.svc/GetAvailableVehicles?BranchID=1&LanguageID=2`, | ||
| { method: 'HEAD', mode: 'cors' }, | ||
| ); | ||
| extensionDetected = resp.ok; | ||
| } catch { | ||
| extensionDetected = false; | ||
| } | ||
| detectionDone = true; |
There was a problem hiding this comment.
fetch() can hang for a long time on some networks; since all production requests now await detectionPromise, this can stall every API call indefinitely. Consider adding an explicit timeout/AbortController (and treating timeout as “no extension”) so requests can proceed via the proxy fallback even if detection never completes.
| try { | |
| const resp = await fetch( | |
| `${TARGET_BASE}/WCF/LSI/LSIBookingServiceV3.svc/GetAvailableVehicles?BranchID=1&LanguageID=2`, | |
| { method: 'HEAD', mode: 'cors' }, | |
| ); | |
| extensionDetected = resp.ok; | |
| } catch { | |
| extensionDetected = false; | |
| } | |
| detectionDone = true; | |
| // Ensure extension detection cannot hang indefinitely; treat timeout as "no extension". | |
| if (typeof AbortController === 'undefined') { | |
| // Fallback for environments without AbortController: behave as before (no explicit timeout). | |
| try { | |
| const resp = await fetch( | |
| `${TARGET_BASE}/WCF/LSI/LSIBookingServiceV3.svc/GetAvailableVehicles?BranchID=1&LanguageID=2`, | |
| { method: 'HEAD', mode: 'cors' }, | |
| ); | |
| extensionDetected = resp.ok; | |
| } catch { | |
| extensionDetected = false; | |
| } | |
| detectionDone = true; | |
| return; | |
| } | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), 5000); | |
| try { | |
| const resp = await fetch( | |
| `${TARGET_BASE}/WCF/LSI/LSIBookingServiceV3.svc/GetAvailableVehicles?BranchID=1&LanguageID=2`, | |
| { method: 'HEAD', mode: 'cors', signal: controller.signal }, | |
| ); | |
| extensionDetected = resp.ok; | |
| } catch { | |
| // Network errors or aborts (including timeout) are treated as "no extension". | |
| extensionDetected = false; | |
| } finally { | |
| clearTimeout(timeoutId); | |
| detectionDone = true; | |
| } |
| const detectionPromise = !import.meta.env.DEV | ||
| ? (async () => { | ||
| try { |
There was a problem hiding this comment.
detectionPromise resolves to void and updates module-level state (extensionDetected/detectionDone). This makes it harder to reason about and requires the extra detectionDone flag. Consider making the promise resolve to the detected boolean (and/or always awaiting the same promise) to reduce shared mutable state and simplify the interceptor logic.
No description provided.