From 46f81e2bae18d5ebc3fedb8d9099475477e1fbbc Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:06:21 +0800 Subject: [PATCH] Fix isNetworkError to match MDN spec: readyState === 4 && status === 0 (#6050) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #4253 The `isNetworkError` function incorrectly classified XHR states as network errors. Per MDN, a network error occurs when a request completes (`readyState === 4`) but has no HTTP status (`status === 0`), indicating network/CORS/file access failures. ## Changes - **Logic fix**: Changed from `(xhr.readyState !== 0 && xhr.readyState !== 4) || xhr.status === 0` to `xhr.readyState === 4 && xhr.status === 0` - **Test update**: Removed invalid test expecting `readyState: 2` to be a network error; added test verifying incomplete requests return `false` ## Example ```typescript // Before: incorrectly treated in-progress requests as network errors isNetworkError({ readyState: 2, status: 0 }) // true ❌ // After: only completed requests with no status are network errors isNetworkError({ readyState: 4, status: 0 }) // true ✓ isNetworkError({ readyState: 2, status: 0 }) // false ✓ ```
Original prompt > Update the isNetworkError function in packages/@uppy/utils/src/isNetworkError.ts to correctly detect network errors according to MDN documentation. The new logic should return true only if xhr.readyState === 4 and xhr.status === 0. The updated implementation should be: > > function isNetworkError(xhr?: XMLHttpRequest): boolean { > if (!xhr) return false > // finished but status is 0 — usually indicates a network/CORS/file error > return xhr.readyState === 4 && xhr.status === 0 > } > > export default isNetworkError > > No other logic changes are needed. If you find related commentary (e.g., outdated comments), clarify as needed.
*This pull request was created as a result of the following prompt from Copilot chat.* > Update the isNetworkError function in packages/@uppy/utils/src/isNetworkError.ts to correctly detect network errors according to MDN documentation. The new logic should return true only if xhr.readyState === 4 and xhr.status === 0. The updated implementation should be: > > function isNetworkError(xhr?: XMLHttpRequest): boolean { > if (!xhr) return false > // finished but status is 0 — usually indicates a network/CORS/file error > return xhr.readyState === 4 && xhr.status === 0 > } > > export default isNetworkError > > No other logic changes are needed. If you find related commentary (e.g., outdated comments), clarify as needed. --- ✨ Let Copilot coding agent [set things up for you](https://github.com/transloadit/uppy/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mifi <402547+mifi@users.noreply.github.com> --- packages/@uppy/utils/src/isNetworkError.test.ts | 14 +++++++------- packages/@uppy/utils/src/isNetworkError.ts | 7 +++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/@uppy/utils/src/isNetworkError.test.ts b/packages/@uppy/utils/src/isNetworkError.test.ts index bfe755d1a..a1a2e8868 100644 --- a/packages/@uppy/utils/src/isNetworkError.test.ts +++ b/packages/@uppy/utils/src/isNetworkError.test.ts @@ -9,12 +9,6 @@ describe('isNetworkError', () => { status: 0, } as any as XMLHttpRequest - const xhrNetworkError2Mock = { - readyState: 2, - responseText: '', - status: 300, - } as any as XMLHttpRequest - const xhrRegularErrorMock = { readyState: 4, responseText: 'Failed', @@ -27,9 +21,15 @@ describe('isNetworkError', () => { status: 200, } as any as XMLHttpRequest + const xhrIncompleteRequestMock = { + readyState: 2, + responseText: '', + status: 0, + } as any as XMLHttpRequest + expect(isNetworkError(xhrNetworkErrorMock)).toEqual(true) - expect(isNetworkError(xhrNetworkError2Mock)).toEqual(true) expect(isNetworkError(xhrRegularErrorMock)).toEqual(false) expect(isNetworkError(xhrNetworkSuccessMock)).toEqual(false) + expect(isNetworkError(xhrIncompleteRequestMock)).toEqual(false) }) }) diff --git a/packages/@uppy/utils/src/isNetworkError.ts b/packages/@uppy/utils/src/isNetworkError.ts index bc806c203..9bce2e3fd 100644 --- a/packages/@uppy/utils/src/isNetworkError.ts +++ b/packages/@uppy/utils/src/isNetworkError.ts @@ -1,8 +1,7 @@ function isNetworkError(xhr?: XMLHttpRequest): boolean { - if (!xhr) { - return false - } - return (xhr.readyState !== 0 && xhr.readyState !== 4) || xhr.status === 0 + if (!xhr) return false + // finished but status is 0 — usually indicates a network/CORS/file error + return xhr.readyState === 4 && xhr.status === 0 } export default isNetworkError