From 8579d2217526a78c2e77b31ee15320519acb19e3 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Fri, 20 Sep 2024 09:29:25 +0100 Subject: [PATCH] Fixed "Editor crashed" error showing for a valid 404 ref https://linear.app/tryghost/issue/ONC-323 - sometimes posts can be deleted by another user or in a different tab but then edited in an old tab that had the post loaded in the editor - in this situation we were displaying our "Editor crashed" error put in place for the rarer situation where the editor is genuinely in a bad state - added an extra conditional for the bad state and a custom error message for the deleted post state --- ghost/admin/app/controllers/lexical-editor.js | 8 ++++- ghost/admin/tests/acceptance/editor-test.js | 32 +++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/ghost/admin/app/controllers/lexical-editor.js b/ghost/admin/app/controllers/lexical-editor.js index 93942639e39e..7a6b2e300633 100644 --- a/ghost/admin/app/controllers/lexical-editor.js +++ b/ghost/admin/app/controllers/lexical-editor.js @@ -699,13 +699,19 @@ export default class LexicalEditorController extends Controller { // into a bad state where it's not saved but the store is treating // it as saved and performing PUT requests with no id. We want to // be noisy about this early to avoid data loss - if (isNotFoundError(error)) { + if (isNotFoundError(error) && !this.post.id) { const notFoundContext = this._getNotFoundErrorContext(); console.error('saveTask failed with 404', notFoundContext); // eslint-disable-line no-console Sentry.captureException(error, {tags: {savePostTask: true}, extra: notFoundContext}); this._showErrorAlert(prevStatus, this.post.status, 'Editor has crashed. Please copy your content and start a new post.'); return; } + if (isNotFoundError(error) && this.post.id) { + const type = this.post.isPage ? 'page' : 'post'; + Sentry.captureMessage(`Attempted to edit deleted ${type}`, {extra: {post_id: this.post.id}}); + this._showErrorAlert(prevStatus, this.post.status, `${capitalizeFirstLetter(type)} has been deleted in a different session. If you need to keep this content, copy it and paste into a new ${type}.`); + return; + } if (error && !isInvalidError(error)) { console.error(error); // eslint-disable-line no-console diff --git a/ghost/admin/tests/acceptance/editor-test.js b/ghost/admin/tests/acceptance/editor-test.js index 235e243bf4ff..73840eefc8b5 100644 --- a/ghost/admin/tests/acceptance/editor-test.js +++ b/ghost/admin/tests/acceptance/editor-test.js @@ -698,7 +698,35 @@ describe('Acceptance: Editor', function () { // no transition alongside the error so this test makes sure that works // and we enter a visible error state rather than letting unsaved changes // pile up and contributing to larger potential data loss. - it('handles 404s from API requests', async function () { + it('handles 404 from invalid PUT API request', async function () { + this.server.put('/posts/', () => { + return new Response(404, {}, { + errors: [ + { + message: 'Resource could not be found.', + errorType: 'NotFoundError', + statusCode: 404 + } + ] + }); + }); + + await visit('/editor/post'); + await waitFor(editorSelector); + + // simulate the bad state where a post.save will trigger a PUT with no id + const controller = this.owner.lookup('controller:lexical-editor'); + controller.post.transitionTo('updated.uncommitted'); + + // this will trigger an autosave which will hit our simulated 404 + await pasteInEditor('Testing'); + + // we should see an error - previously this was failing silently + // error message comes from editor's own handling rather than our generic API error fallback + expect(find('.gh-alert-content')).to.have.trimmed.text('Saving failed: Editor has crashed. Please copy your content and start a new post.'); + }); + + it('handles 404 from valid PUT API request', async function () { // this doesn't match what we're actually seeing in the above mentioned // bug state but it's a good enough simulation for testing our error handler this.server.put('/posts/:id/', () => { @@ -724,7 +752,7 @@ describe('Acceptance: Editor', function () { // we should see an error - previously this was failing silently // error message comes from editor's own handling rather than our generic API error fallback - expect(find('.gh-alert-content')).to.have.trimmed.text('Saving failed: Editor has crashed. Please copy your content and start a new post.'); + expect(find('.gh-alert-content')).to.contain.text('Post has been deleted in a different session'); }); }); });