diff --git a/packages/api-fetch/CHANGELOG.md b/packages/api-fetch/CHANGELOG.md index 5fb351605b48ac..836856d9572b20 100644 --- a/packages/api-fetch/CHANGELOG.md +++ b/packages/api-fetch/CHANGELOG.md @@ -6,6 +6,10 @@ - Raise the minimum required Node.js version to `>=22.13.0`, for stable `require(esm)` support ([#84313](https://github.com/WordPress/gutenberg/pull/84313)). +### Enhancements + +- Include the response's status as `data.status` in `invalid_json` errors, and in error responses that have no `data` ([#85157](https://github.com/WordPress/gutenberg/pull/85157)). + ## 7.57.0 (2026-10-07) ## 7.56.0 (2026-09-23) diff --git a/packages/api-fetch/src/test/index.jsdom.test.js b/packages/api-fetch/src/test/index.jsdom.test.js index f848e55c43c88e..38eda68cc2283a 100644 --- a/packages/api-fetch/src/test/index.jsdom.test.js +++ b/packages/api-fetch/src/test/index.jsdom.test.js @@ -131,6 +131,27 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'bad_request', message: 'Bad Request', + data: { status: 400 }, + } ); + } ); + + it( 'should keep the data of an error response', async () => { + globalThis.fetch.mockResolvedValue( { + ok: false, + status: 500, + async json() { + return { + code: 'db_update_error', + message: 'Could not update post in the database.', + data: 'Table does not exist', + }; + }, + } ); + + await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { + code: 'db_update_error', + message: 'Could not update post in the database.', + data: 'Table does not exist', } ); } ); @@ -146,6 +167,7 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'invalid_json', message: 'The response is not a valid JSON response.', + data: { status: 200 }, } ); } ); @@ -161,6 +183,7 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'invalid_json', message: 'The response is not a valid JSON response.', + data: { status: 200 }, } ); } ); @@ -234,6 +257,7 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'invalid_json', message: 'The response is not a valid JSON response.', + data: { status: 200 }, } ); } ); @@ -245,6 +269,7 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'invalid_json', message: 'The response is not a valid JSON response.', + data: { status: 500 }, } ); } ); @@ -357,6 +382,7 @@ describe( 'apiFetch', () => { await expect( apiFetch( { path: '/random' } ) ).rejects.toEqual( { code: 'rest_cookie_invalid_nonce', + data: { status: 403 }, } ); } ); diff --git a/packages/api-fetch/src/utils/response.ts b/packages/api-fetch/src/utils/response.ts index 358856718470a1..9ff3e0716a8ee3 100644 --- a/packages/api-fetch/src/utils/response.ts +++ b/packages/api-fetch/src/utils/response.ts @@ -32,6 +32,7 @@ async function parseJsonAndNormalizeError( throw { code: 'invalid_json', message: __( 'The response is not a valid JSON response.' ), + data: { status: response.status }, }; } } @@ -75,5 +76,16 @@ export async function parseAndThrowError( } // Parse the response JSON and throw it as an error. - throw await parseJsonAndNormalizeError( response ); + const error = await parseJsonAndNormalizeError( response ); + + // Like WordPress's own REST errors, an error carries its status in `data`. + if ( + error && + typeof error === 'object' && + ! Array.isArray( error ) && + error.data === undefined + ) { + throw { ...error, data: { status: response.status } }; + } + throw error; } diff --git a/packages/editor/src/store/actions.js b/packages/editor/src/store/actions.js index d72fce8de7db81..4e09dc925b92ef 100644 --- a/packages/editor/src/store/actions.js +++ b/packages/editor/src/store/actions.js @@ -23,7 +23,7 @@ import { doActionAsync, } from '@wordpress/hooks'; import { store as preferencesStore } from '@wordpress/preferences'; -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; import { localAutosaveSet } from './local-autosave'; import { getNotificationArgumentsForSaveSuccess, @@ -39,9 +39,13 @@ import { AUTOSAVE_RETRY_SLOT } from './constants'; const { getEntitySnapshot } = unlock( coreDataPrivateApis ); // Error codes whose message is generated by the client rather than returned by -// the server. Their messages restate what the notice already says, so there are -// no details worth disclosing. -const CLIENT_GENERATED_ERROR_CODES = [ 'offline_error', 'fetch_error' ]; +// the server. Their messages add nothing to the notice, so they are not shown as +// details. +const CLIENT_GENERATED_ERROR_CODES = [ + 'offline_error', + 'fetch_error', + 'invalid_json', +]; // Block-level elements whose end is a line break once markup is removed, so two // sentences in separate paragraphs don't run together. @@ -408,11 +412,26 @@ export const savePost = // A PHP fatal error's message, with its stack trace, comes in // `data.error`, apart from the generic message WordPress shows. const phpError = error.data?.error?.message; - const details = [ + const status = error.data?.status; + const message = error.message && ! CLIENT_GENERATED_ERROR_CODES.includes( error.code ) ? getFailureDetails( error.message ) + : ''; + const summary = [ + Number.isInteger( status ) && status >= 400 + ? sprintf( + /* translators: %d: HTTP status code, such as 500. */ + __( 'Error code %d.' ), + status + ) : '', + message, + ] + .filter( Boolean ) + .join( ' ' ); + const details = [ + summary, typeof phpError === 'string' ? phpError.trim() : '', ] .filter( Boolean ) diff --git a/packages/editor/src/store/test/actions.jsdom.test.js b/packages/editor/src/store/test/actions.jsdom.test.js index 553f314953d7fa..0edc02a8d08eda 100644 --- a/packages/editor/src/store/test/actions.jsdom.test.js +++ b/packages/editor/src/store/test/actions.jsdom.test.js @@ -427,7 +427,7 @@ describe( 'Post actions', () => { }, }, }, - 'There has been a critical error on this website.\n\nUncaught Error: Call to undefined function f() in /var/www/html/a.php:2\nStack trace:\n#0 {main}\n thrown', + 'Error code 500. There has been a critical error on this website.\n\nUncaught Error: Call to undefined function f() in /var/www/html/a.php:2\nStack trace:\n#0 {main}\n thrown', ], [ 'ignores a PHP error message that is not a string', @@ -436,7 +436,25 @@ describe( 'Post actions', () => { message: 'Server error.', data: { status: 500, error: { message: [ 'unexpected' ] } }, }, - 'Server error.', + 'Error code 500. Server error.', + ], + [ + 'shows the status of a response that is not JSON', + { + code: 'invalid_json', + message: 'The response is not a valid JSON response.', + data: { status: 502 }, + }, + 'Error code 502.', + ], + [ + 'shows no details for a successful response that is not JSON', + { + code: 'invalid_json', + message: 'The response is not a valid JSON response.', + data: { status: 200 }, + }, + undefined, ], ] )( '%s', async ( _description, serverError, expectedDetails ) => { const post = { diff --git a/packages/editor/src/store/utils/notice-builder.js b/packages/editor/src/store/utils/notice-builder.js index 0713cdee670900..8c15714a22bf8a 100644 --- a/packages/editor/src/store/utils/notice-builder.js +++ b/packages/editor/src/store/utils/notice-builder.js @@ -5,6 +5,10 @@ const AUTO_SAVE_FAILURE_NOTICE = __( 'Auto-save failed. You can also save manually.' ); +const AUTO_SAVE_NO_RESPONSE_FAILURE_NOTICE = __( + 'Auto-save failed because there was no response from the server. You can also save manually.' +); + const AUTO_SAVE_OFFLINE_FAILURE_NOTICE = __( 'Auto-save failed because you were offline. You can also save manually once you’re back online.' ); @@ -150,7 +154,10 @@ export function getNotificationArgumentsForSaveFail( data ) { : messages.default; if ( options?.isAutosave ) { - noticeMessage = AUTO_SAVE_FAILURE_NOTICE; + noticeMessage = + error.code === 'fetch_error' + ? AUTO_SAVE_NO_RESPONSE_FAILURE_NOTICE + : AUTO_SAVE_FAILURE_NOTICE; } return [ noticeMessage, { id } ]; diff --git a/packages/editor/src/store/utils/test/notice-builder.js b/packages/editor/src/store/utils/test/notice-builder.js index 58036303e017d3..244baa47b3b94b 100644 --- a/packages/editor/src/store/utils/test/notice-builder.js +++ b/packages/editor/src/store/utils/test/notice-builder.js @@ -158,6 +158,16 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { ], { isAutosave: true }, ], + [ + 'when the autosave got no response', + 'fetch_error', + [ 'publish', 'publish' ], + [ + 'Auto-save failed because there was no response from the server. You can also save manually.', + { id: 'editor-autosave' }, + ], + { isAutosave: true }, + ], ].forEach( ( [ description, diff --git a/test/e2e/specs/editor/various/autosave-failure-notice.spec.js b/test/e2e/specs/editor/various/autosave-failure-notice.spec.js index 0a44bb78bdeecf..5da62979b95481 100644 --- a/test/e2e/specs/editor/various/autosave-failure-notice.spec.js +++ b/test/e2e/specs/editor/various/autosave-failure-notice.spec.js @@ -5,6 +5,9 @@ const { const AUTOSAVE_FAILED = 'Auto-save failed. You can also save manually.'; +const AUTOSAVE_FAILED_NO_RESPONSE = + 'Auto-save failed because there was no response from the server. You can also save manually.'; + const UPDATING_FAILED_OFFLINE = 'Updating failed because you were offline. Please verify your connection and try again.'; @@ -82,78 +85,84 @@ test.describe( 'Autosave failure notice', () => { // WordPress version, so only its start is checked. withDetails( expect.stringMatching( - /^There has been a critical error on this website\.\nLearn more about troubleshooting WordPress\. \(https:\/\/wordpress\.org\/documentation\/article\/faq-troubleshooting\/\)\n\nUncaught Error: Call to undefined function gutenberg_test_autosave_undefined_function\(\) in .+\nStack trace:\n#0 / + /^Error code 500\. There has been a critical error on this website\.\nLearn more about troubleshooting WordPress\. \(https:\/\/wordpress\.org\/documentation\/article\/faq-troubleshooting\/\)\n\nUncaught Error: Call to undefined function gutenberg_test_autosave_undefined_function\(\) in .+\nStack trace:\n#0 / ) ), ], [ 'database connection lost', 'database-connection-lost', - withDetails( 'Error establishing a database connection' ), + withDetails( + 'Error code 500. Error establishing a database connection' + ), ], [ 'error message containing a link', 'link-in-message', - withDetails( 'Blocked. Learn more (https://example.com).' ), + withDetails( + 'Error code 403. Blocked. Learn more (https://example.com).' + ), ], - // TODO: Say the server could not be reached, and retry right away. + // TODO: Retry right away. [ 'connection dropped before a response', 'dropped-connection', - AUTOSAVE_FAILED, + AUTOSAVE_FAILED_NO_RESPONSE, + ], + [ + 'error without a message', + 'no-message', + withDetails( 'Error code 500.' ), ], - // TODO: Show the status code. - [ 'error without a message', 'no-message', AUTOSAVE_FAILED ], [ 'error with a plain text message', 'plain-message', - withDetails( 'Something went wrong on the server.' ), + withDetails( + 'Error code 500. Something went wrong on the server.' + ), ], [ 'permission denied', 'permission', - withDetails( 'Sorry, you are not allowed to edit this post.' ), + withDetails( + 'Error code 403. Sorry, you are not allowed to edit this post.' + ), ], + // The error's `data` is the database error, so there is no status in it. [ 'database write failed', 'database-write', withDetails( 'Could not update post in the database.' ), ], - // TODO: Show the status code (502) instead. - [ - 'HTML error page', - 'bad-gateway', - withDetails( 'The response is not a valid JSON response.' ), - ], - // TODO: Show the status code (429) instead. + [ 'HTML error page', 'bad-gateway', withDetails( 'Error code 502.' ) ], [ 'too many requests', 'too-many-requests', - withDetails( 'The response is not a valid JSON response.' ), + withDetails( 'Error code 429.' ), ], + // The error is printed before the headers, so the status stays 200. // TODO: Show the PHP error the response includes in `data.error`. [ 'PHP fatal error printed before the JSON', 'fatal-error-displayed', - withDetails( 'The response is not a valid JSON response.' ), + AUTOSAVE_FAILED, ], // TODO: Say the session may have expired. [ 'redirected to the login page', 'redirect-to-login', - withDetails( 'The response is not a valid JSON response.' ), + AUTOSAVE_FAILED, ], - // TODO: Say the server could not be reached. [ 'redirected to another site', 'redirect-to-other-site', - AUTOSAVE_FAILED, + AUTOSAVE_FAILED_NO_RESPONSE, ], [ 'maintenance mode', 'maintenance', withDetails( - 'Briefly unavailable for scheduled maintenance. Check back in a minute.' + 'Error code 503. Briefly unavailable for scheduled maintenance. Check back in a minute.' ), ], // TODO: Report the failure; the autosave fails without a notice. @@ -208,7 +217,7 @@ test.describe( 'Autosave failure notice', () => { await autosaveFailure.autosave(); // TODO: Say the session expired and offer to log in again. expect( await autosaveFailure.getNotices() ).toEqual( [ - withDetails( 'Cookie check failed' ), + withDetails( 'Error code 403. Cookie check failed' ), ] ); } ); @@ -221,7 +230,7 @@ test.describe( 'Autosave failure notice', () => { const title = await autosaveFailure.autosave(); // TODO: Don't report a failure; the autosave was stored. expect( await autosaveFailure.getNotices() ).toEqual( [ - withDetails( 'The response is not a valid JSON response.' ), + AUTOSAVE_FAILED, ] ); const postId = await page.evaluate( () => @@ -298,11 +307,11 @@ test.describe( 'Autosave failure notice', () => { .locator( '.notices-inline-notices__notice' ); await expect( notice ).toContainText( new RegExp( - `${ AUTOSAVE_FAILED.replaceAll( '.', '\\.' ) } Retrying in (10|9) seconds\\.` + `${ AUTOSAVE_FAILED_NO_RESPONSE.replaceAll( '.', '\\.' ) } Retrying in (10|9) seconds\\.` ) ); await expect( notice ).toContainText( - `${ AUTOSAVE_FAILED } Retrying in 7 seconds.` + `${ AUTOSAVE_FAILED_NO_RESPONSE } Retrying in 7 seconds.` ); } ); @@ -323,7 +332,7 @@ test.describe( 'Autosave failure notice', () => { .getByRole( 'region', { name: 'Editor content' } ) .locator( '.notices-inline-notices__notice' ); await expect( notice ).toContainText( - `${ AUTOSAVE_FAILED } Retrying…` + `${ AUTOSAVE_FAILED_NO_RESPONSE } Retrying…` ); await expect( notice ).toHaveCount( 0 ); } ); @@ -355,7 +364,7 @@ test.describe( 'Autosave failure notice', () => { } } expect( await autosaveFailure.getNotices() ).toEqual( [ - AUTOSAVE_FAILED, + AUTOSAVE_FAILED_NO_RESPONSE, ] ); } ); } @@ -366,7 +375,7 @@ test.describe( 'Autosave failure notice', () => { await autosaveFailure.fail( 'dropped-connection' ); await autosaveFailure.autosave(); expect( await autosaveFailure.getNotices() ).toEqual( [ - AUTOSAVE_FAILED, + AUTOSAVE_FAILED_NO_RESPONSE, ] ); await autosaveFailure.stopFailing(); @@ -381,7 +390,7 @@ test.describe( 'Autosave failure notice', () => { await autosaveFailure.fail( 'dropped-connection' ); await autosaveFailure.autosave(); expect( await autosaveFailure.getNotices() ).toEqual( [ - AUTOSAVE_FAILED, + AUTOSAVE_FAILED_NO_RESPONSE, ] ); await autosaveFailure.stopFailing(); @@ -400,7 +409,7 @@ test.describe( 'Autosave failure notice', () => { await autosaveFailure.fail( 'dropped-connection' ); await autosaveFailure.autosave(); expect( await autosaveFailure.getNotices() ).toEqual( [ - AUTOSAVE_FAILED, + AUTOSAVE_FAILED_NO_RESPONSE, ] ); await context.setOffline( true );