From 073b7661940a7bf25d7e322e9caed00ede28281c Mon Sep 17 00:00:00 2001 From: Miguel Fonseca <150562+mcsf@users.noreply.github.com> Date: Fri, 29 Aug 2025 17:24:49 +0100 Subject: [PATCH 1/3] Check navigator.onLine to improve failure notices When handling a failed request, let api-fetch infer from the value of `navigator.onLine` whether the failure likely stems from the fact that the user is disconnected from the network. This still leaves out other kinds of network error, which right now will still trigger the fallback "Unexpected error" messaging. The spec for `Window.fetch` does not prescribe a standard error for network errors, meaning that browsers each implement their own messaging. --- packages/api-fetch/src/index.ts | 14 +++++++++--- .../editor/src/store/utils/notice-builder.js | 22 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/packages/api-fetch/src/index.ts b/packages/api-fetch/src/index.ts index a5cd76253f3ebd..b10373b3d9c63b 100644 --- a/packages/api-fetch/src/index.ts +++ b/packages/api-fetch/src/index.ts @@ -114,11 +114,19 @@ const defaultFetchHandler: FetchHandler = ( nextOptions ) => { throw err; } - // Otherwise, there is most likely no network connection. - // Unfortunately the message might depend on the browser. + // If the browser reports being offline, we'll just assume that + // this is why the request failed. + if ( ! window.navigator.onLine ) { + throw { + code: 'offline_error', + message: __( 'You are offline.' ), + }; + } + + // Hard to diagnose further due to how Window.fetch reports errors. throw { code: 'fetch_error', - message: __( 'You are probably offline.' ), + message: __( 'An unexpected error occurred.' ), }; } ); diff --git a/packages/editor/src/store/utils/notice-builder.js b/packages/editor/src/store/utils/notice-builder.js index f0d1f4329540c8..1b03ca7a52f609 100644 --- a/packages/editor/src/store/utils/notice-builder.js +++ b/packages/editor/src/store/utils/notice-builder.js @@ -89,15 +89,31 @@ export function getNotificationArgumentsForSaveFail( data ) { const publishStatus = [ 'publish', 'private', 'future' ]; const isPublished = publishStatus.indexOf( post.status ) !== -1; - // If the post was being published, we show the corresponding publish error message - // Unless we publish an "updating failed" message. + + if ( error.code === 'offline_error' ) { + const messages = { + publish: __( 'Publishing failed because you were offline.' ), + private: __( 'Publishing failed because you were offline.' ), + future: __( 'Scheduling failed because you were offline.' ), + default: __( 'Updating failed because you were offline.' ), + }; + + const noticeMessage = + ! isPublished && edits.status in messages + ? messages[ edits.status ] + : messages.default; + + return [ noticeMessage, { id: 'editor-save' } ]; + } + const messages = { publish: __( 'Publishing failed.' ), private: __( 'Publishing failed.' ), future: __( 'Scheduling failed.' ), }; + let noticeMessage = - ! isPublished && publishStatus.indexOf( edits.status ) !== -1 + ! isPublished && edits.status in messages ? messages[ edits.status ] : __( 'Updating failed.' ); From aff32bef1c68e39d2c5fc11ad90f354bc9777800 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca <150562+mcsf@users.noreply.github.com> Date: Mon, 1 Sep 2025 11:37:04 +0100 Subject: [PATCH 2/3] Address feedback Props jsnajdr, tyxla --- packages/api-fetch/src/index.ts | 8 ++++++-- packages/editor/src/store/utils/notice-builder.js | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/api-fetch/src/index.ts b/packages/api-fetch/src/index.ts index b10373b3d9c63b..683bdba7a291f2 100644 --- a/packages/api-fetch/src/index.ts +++ b/packages/api-fetch/src/index.ts @@ -119,14 +119,18 @@ const defaultFetchHandler: FetchHandler = ( nextOptions ) => { if ( ! window.navigator.onLine ) { throw { code: 'offline_error', - message: __( 'You are offline.' ), + message: __( + 'Unable to connect. Please check your Internet connection.' + ), }; } // Hard to diagnose further due to how Window.fetch reports errors. throw { code: 'fetch_error', - message: __( 'An unexpected error occurred.' ), + message: __( + 'Could not get a valid response from the server.' + ), }; } ); diff --git a/packages/editor/src/store/utils/notice-builder.js b/packages/editor/src/store/utils/notice-builder.js index 1b03ca7a52f609..496af4f8b55541 100644 --- a/packages/editor/src/store/utils/notice-builder.js +++ b/packages/editor/src/store/utils/notice-builder.js @@ -110,12 +110,13 @@ export function getNotificationArgumentsForSaveFail( data ) { publish: __( 'Publishing failed.' ), private: __( 'Publishing failed.' ), future: __( 'Scheduling failed.' ), + default: __( 'Updating failed.' ), }; let noticeMessage = ! isPublished && edits.status in messages ? messages[ edits.status ] - : __( 'Updating failed.' ); + : messages.default; // Check if message string contains HTML. Notice text is currently only // supported as plaintext, and stripping the tags may muddle the meaning. From b0c0b85da702dd1af3e81440087af3471d4c9cd1 Mon Sep 17 00:00:00 2001 From: Miguel Fonseca <150562+mcsf@users.noreply.github.com> Date: Mon, 1 Sep 2025 14:31:10 +0100 Subject: [PATCH 3/3] Update tests --- packages/api-fetch/src/test/index.js | 2 +- packages/media-utils/src/utils/test/upload-media.ts | 4 ++-- test/e2e/specs/editor/various/autosave.spec.js | 2 +- test/e2e/specs/editor/various/change-detection.spec.js | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/api-fetch/src/test/index.js b/packages/api-fetch/src/test/index.js index 3607f9e2d2eca5..0a95992b5708b1 100644 --- a/packages/api-fetch/src/test/index.js +++ b/packages/api-fetch/src/test/index.js @@ -168,7 +168,7 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'fetch_error', - message: 'You are probably offline.', + message: 'Could not get a valid response from the server.', } ); } ); diff --git a/packages/media-utils/src/utils/test/upload-media.ts b/packages/media-utils/src/utils/test/upload-media.ts index 66cdeefc53039f..8d827239f3d1f2 100644 --- a/packages/media-utils/src/utils/test/upload-media.ts +++ b/packages/media-utils/src/utils/test/upload-media.ts @@ -215,7 +215,7 @@ describe( 'uploadMedia', () => { ( uploadToServer as jest.Mock ).mockImplementation( () => { throw { code: 'fetch_error', - message: 'You are probably offline.', + message: 'Could not get a valid response from the server.', }; } ); @@ -229,7 +229,7 @@ describe( 'uploadMedia', () => { expect( onError ).toHaveBeenCalledWith( new UploadError( { code: 'GENERAL', - message: 'You are probably offline.', + message: 'Could not get a valid response from the server.', file: imageFile, } ) ); diff --git a/test/e2e/specs/editor/various/autosave.spec.js b/test/e2e/specs/editor/various/autosave.spec.js index 01d53f0bf2d085..39c9f153b39b85 100644 --- a/test/e2e/specs/editor/various/autosave.spec.js +++ b/test/e2e/specs/editor/various/autosave.spec.js @@ -270,7 +270,7 @@ test.describe( 'Autosave', () => { await expect( page.locator( '.components-notice__content' ) - ).toContainText( 'Updating failed. You are probably offline.' ); + ).toContainText( 'Updating failed because you were offline.' ); expect( await page.evaluate( () => window.sessionStorage.length ) ).toBe( 1 ); diff --git a/test/e2e/specs/editor/various/change-detection.spec.js b/test/e2e/specs/editor/various/change-detection.spec.js index 12d13b4890e368..2b31b65e99b44e 100644 --- a/test/e2e/specs/editor/various/change-detection.spec.js +++ b/test/e2e/specs/editor/various/change-detection.spec.js @@ -243,7 +243,7 @@ test.describe( 'Change detection', () => { await expect( page .getByRole( 'region', { name: 'Editor content' } ) - .getByText( 'Updating failed. You are probably offline.' ) + .getByText( 'Updating failed because you were offline.' ) ).toBeVisible(); await expect( page