From bf87a20460ee1614b8ff92ac875afeb41a06e98b Mon Sep 17 00:00:00 2001 From: yushihang Date: Tue, 11 Mar 2025 20:42:55 +0800 Subject: [PATCH 1/2] fix: race condition in abort handling Timeline of the race condition: Time 0: - AbortController instance A is created - Request A is initiated with instance A Time 1: (User switches tab, making it invisible) - onVisibilityChange() is triggered - abort() is called on instance A - create() is queued to execute when tab becomes visible Time 2: (User switches back, making tab visible) - create() executes - AbortController instance B is created - Request B is initiated with instance B Time 3: - Request A's abort error is caught in catch block - Bug: Code checks instance B's abort status instead of instance A's - Since instance B is not aborted, onerror callback is wrongly triggered - This leads to an unnecessary retry attempt Fix: Store AbortController instance in local scope to ensure error handling uses the correct instance: Before: ```ts curRequestController = new AbortController(); // Later in catch block if (!curRequestController.signal.aborted) ``` After: ```ts const currentController = new AbortController(); curRequestController = currentController; // Later in catch block if (!currentController.signal.aborted) ``` --- src/fetch.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fetch.ts b/src/fetch.ts index 162ea45..1c39417 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -100,7 +100,8 @@ export function fetchEventSource(input: RequestInfo, { const fetch = inputFetch ?? window.fetch; const onopen = inputOnOpen ?? defaultOnOpen; async function create() { - curRequestController = new AbortController(); + const currentController = new AbortController(); + curRequestController = currentController; try { const response = await fetch(input, { ...rest, @@ -126,7 +127,7 @@ export function fetchEventSource(input: RequestInfo, { dispose(); resolve(); } catch (err) { - if (!curRequestController.signal.aborted) { + if (!currentController.signal.aborted) { // if we haven't aborted the request ourselves: try { // check if we need to retry: From 617fa0a417cb409359eedbc137f4b091bbecaa03 Mon Sep 17 00:00:00 2001 From: Jokerlee <364086465@qq.com> Date: Tue, 11 Mar 2025 21:25:02 +0800 Subject: [PATCH 2/2] Add comments --- src/fetch.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/fetch.ts b/src/fetch.ts index 1c39417..c2721a5 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -100,6 +100,17 @@ export function fetchEventSource(input: RequestInfo, { const fetch = inputFetch ?? window.fetch; const onopen = inputOnOpen ?? defaultOnOpen; async function create() { + // Store the AbortController instance in function scope to prevent race conditions + // This ensures that when error handling occurs, we check the abort status + // of the correct controller instance that was associated with this specific request, + // rather than potentially checking a new controller created by a subsequent request. + // + // Race condition example: + // 1. Request A starts with Controller A + // 2. Tab becomes hidden -> Controller A is aborted + // 3. Tab becomes visible -> Request B starts with Controller B + // 4. Request A's error handler executes + // Without this fix, it would check Controller B's status instead of A's const currentController = new AbortController(); curRequestController = currentController; try {