Added full-page refresh when new post route is in a bad state

ref https://linear.app/tryghost/issue/ONC-323

When the store gets into a bad state for new posts that causes saves to fail we can detect that by looking at the `model.isNew` property. Currently our best approach to fix this state is to restart the app.

- added a `didTransition()` hook to our `lexical-edit.new` route
  - detects the bad state, logs the error, and triggers a browser refresh
  - logs with a `recreatedPostIsGood` property that will let us know if we could instead just try recreating the post and avoiding a full refresh (so far we have no reproduction case so we need to learn what we can)
- added `sinon-chai` dependency for better assertions on spies/stubs
- added `sentry-testkit` dependency so we can test our Sentry integration calls
  - we can't use sinon for these calls because of the way Sentry's es6 imports work
- extracted our full Sentry config object generation to a util function so it can be re-used in unit tests
  - updated our integrations list to disable the default `dedupe` integration because it can cause very unexpected/difficult to debug test failures when you're asserting using `sentry-testkit`

ghost/admin/app/controllers/lexical-editor.js

@@ -196,7 +196,7 @@ export default class LexicalEditorController extends Controller {
     _pushPostState() {
         const post = this.post;
 
-        if (!post) {
+        if (!post || !post.currentState) {
             return;
         }
 

ghost/admin/app/routes/application.js

@@ -7,9 +7,7 @@ import Route from '@ember/routing/route';
 import ShortcutsRoute from 'ghost-admin/mixins/shortcuts-route';
 import ctrlOrCmd from 'ghost-admin/utils/ctrl-or-cmd';
 import windowProxy from 'ghost-admin/utils/window-proxy';
-import {Debug} from '@sentry/integrations';
-import {Replay} from '@sentry/replay';
-import {beforeSend} from 'ghost-admin/utils/sentry';
+import {getSentryConfig} from '../utils/sentry';
 import {importComponent} from '../components/admin-x/admin-x-component';
 import {inject} from 'ghost-admin/decorators/inject';
 import {
@@ -184,68 +182,7 @@ export default Route.extend(ShortcutsRoute, {
         // init Sentry here rather than app.js so that we can use API-supplied
         // sentry_dsn and sentry_env rather than building it into release assets
         if (this.config.sentry_dsn) {
-            const sentryConfig = {
-                dsn: this.config.sentry_dsn,
-                environment: this.config.sentry_env,
-                release: `ghost@${this.config.version}`,
-                beforeSend,
-                ignoreErrors: [
-                    // Browser autoplay policies (this regex covers a few)
-                    /The play\(\) request was interrupted.*/,
-                    /The request is not allowed by the user agent or the platform in the current context/,
-
-                    // Network errors that we don't control
-                    /Server was unreachable/,
-                    /NetworkError when attempting to fetch resource./,
-                    /Failed to fetch/,
-                    /Load failed/,
-                    /The operation was aborted./,
-
-                    // TransitionAborted errors surface from normal application behaviour
-                    // - https://github.com/emberjs/ember.js/issues/12505
-                    /^TransitionAborted$/,
-                    // ResizeObserver loop errors occur often from extensions and
-                    // embedded content, generally harmless and not useful to report
-                    /^ResizeObserver loop completed with undelivered notifications/,
-                    /^ResizeObserver loop limit exceeded/,
-                    // When tasks in ember-concurrency are canceled, they sometimes lead to unhandled Promise rejections
-                    // This doesn't affect the application and is not useful to report
-                    // - http://ember-concurrency.com/docs/cancelation
-                    'TaskCancelation'
-                ],
-                integrations: [],
-                beforeBreadcrumb(breadcrumb) {
-                    // ignore breadcrumbs for event tracking to reduce noise in error reports
-                    if (breadcrumb.category === 'http' && breadcrumb.data?.url?.match(/\/e\.ghost\.org|plausible\.io/)) {
-                        return null;
-                    }
-                    return breadcrumb;
-                }
-            };
-
-            try {
-                // Session Replay on errors
-                // Docs: https://docs.sentry.io/platforms/javascript/session-replay
-                sentryConfig.replaysOnErrorSampleRate = 0.5;
-                sentryConfig.integrations.push(
-                    // Replace with `Sentry.replayIntegration()` once we've migrated to @sentry/ember 8.x
-                    // Docs: https://docs.sentry.io/platforms/javascript/migration/v7-to-v8/#removal-of-sentryreplay-package
-                    new Replay({
-                        mask: ['.koenig-lexical', '.gh-dashboard'],
-                        unmask: ['[role="menu"]', '[data-testid="settings-panel"]', '.gh-nav'],
-                        maskAllText: false,
-                        maskAllInputs: true,
-                        blockAllMedia: true
-                    })
-                );
-            } catch (e) {
-                // no-op, Session Replay is not critical
-                console.error('Error enabling Sentry Replay:', e); // eslint-disable-line no-console
-            }
-
-            if (this.config.sentry_env === 'development') {
-                sentryConfig.integrations.push(new Debug());
-            }
+            const sentryConfig = getSentryConfig(this.config.sentry_dsn, this.config.sentry_env, this.config.version);
             Sentry.init(sentryConfig);
         }
 

ghost/admin/app/routes/lexical-editor/new.js

@@ -1,4 +1,8 @@
+import * as Sentry from '@sentry/ember';
 import AuthenticatedRoute from 'ghost-admin/routes/authenticated';
+import windowProxy from 'ghost-admin/utils/window-proxy';
+import {action} from '@ember/object';
+import {scheduleOnce} from '@ember/runloop';
 
 export default class NewRoute extends AuthenticatedRoute {
     model(params, transition) {
@@ -13,12 +17,42 @@ export default class NewRoute extends AuthenticatedRoute {
     }
 
     // there's no specific controller for this route, instead all editor
-    // handling is done on the editor route/controler
+    // handling is done on the editor route/controller
     setupController(controller, newPost) {
+        // logging here because at this stage the controller has definitely not
+        // had a hand in doing anything to the post
+        if (!newPost.isNew) {
+            console.error('New post route did not generate a new model'); // eslint-disable-line no-console
+        }
+
         let editor = this.controllerFor('lexical-editor');
         editor.setPost(newPost);
     }
 
+    // We've seen rare occurrences of getting a newly created model with
+    // isNew: false which will result in errors when saving because it tries
+    // a PUT request with no id. This is a safety check to get out of that bad
+    // state to avoid potential data loss from failed post creation saves.
+    //
+    // Because we trigger a browser refresh to get out of this state we need to
+    // have completed the transition so the refresh occurs on the right URL which
+    // is why we do this in `didTransition` rather than earlier hooks.
+    @action
+    didTransition() {
+        const controller = this.controllerFor('lexical-editor');
+        const post = controller.post;
+
+        if (!post.isNew) {
+            const newPostAttempt = this.store?.createRecord('post', {authors: [this.session.user]});
+
+            console.error('New post route transitioned with post.isNew=false', {recreatedPostIsGood: newPostAttempt.isNew}); // eslint-disable-line no-console
+            Sentry.captureMessage('New post route transitioned with post.isNew=false', {tags: {savePostTask: true}, extra: {recreatedPostIsGood: newPostAttempt.isNew}});
+
+            // still need to schedule the refresh to allow the transition to fully complete and URL to update
+            scheduleOnce('afterRender', this, windowProxy.reload);
+        }
+    }
+
     buildRouteInfoMetadata() {
         return {
             mainClasses: ['editor-new']

ghost/admin/app/utils/sentry.js

@@ -1,6 +1,99 @@
-import {
-    isAjaxError
-} from 'ember-ajax/errors';
+import {Debug} from '@sentry/integrations';
+import {Replay} from '@sentry/replay';
+import {isAjaxError} from 'ember-ajax/errors';
+
+export function getSentryConfig(dsn, environment, appVersion, transport) {
+    const extraIntegrations = [];
+
+    const config = {
+        dsn,
+        transport,
+        environment,
+        release: `ghost@${appVersion}`,
+        beforeSend,
+        ignoreErrors: [
+            // Browser autoplay policies (this regex covers a few)
+            /The play\(\) request was interrupted.*/,
+            /The request is not allowed by the user agent or the platform in the current context/,
+
+            // Network errors that we don't control
+            /Server was unreachable/,
+            /NetworkError when attempting to fetch resource./,
+            /Failed to fetch/,
+            /Load failed/,
+            /The operation was aborted./,
+
+            // TransitionAborted errors surface from normal application behaviour
+            // - https://github.com/emberjs/ember.js/issues/12505
+            /^TransitionAborted$/,
+            // ResizeObserver loop errors occur often from extensions and
+            // embedded content, generally harmless and not useful to report
+            /^ResizeObserver loop completed with undelivered notifications/,
+            /^ResizeObserver loop limit exceeded/,
+            // When tasks in ember-concurrency are canceled, they sometimes lead to unhandled Promise rejections
+            // This doesn't affect the application and is not useful to report
+            // - http://ember-concurrency.com/docs/cancelation
+            'TaskCancelation'
+        ],
+        integrations: function (integrations) {
+            // integrations will be all default integrations
+            const defaultIntegrations = integrations.filter((integration) => {
+                // Don't dedupe events when testing
+                if (environment === 'testing' && integration.name === 'Dedupe') {
+                    return false;
+                }
+
+                return true;
+            });
+
+            return [...defaultIntegrations, ...extraIntegrations];
+        },
+        beforeBreadcrumb(breadcrumb) {
+            // ignore breadcrumbs for event tracking to reduce noise in error reports
+            if (breadcrumb.category === 'http' && breadcrumb.data?.url?.match(/\/e\.ghost\.org|plausible\.io/)) {
+                return null;
+            }
+            return breadcrumb;
+        }
+    };
+
+    if (environment !== 'testing') {
+        try {
+            // Session Replay on errors
+            // Docs: https://docs.sentry.io/platforms/javascript/session-replay
+            config.replaysOnErrorSampleRate = 0.5;
+            extraIntegrations.push(
+                // Replace with `Sentry.replayIntegration()` once we've migrated to @sentry/ember 8.x
+                // Docs: https://docs.sentry.io/platforms/javascript/migration/v7-to-v8/#removal-of-sentryreplay-package
+                new Replay({
+                    mask: ['.koenig-lexical', '.gh-dashboard'],
+                    unmask: ['[role="menu"]', '[data-testid="settings-panel"]', '.gh-nav'],
+                    maskAllText: false,
+                    maskAllInputs: true,
+                    blockAllMedia: true
+                })
+            );
+        } catch (e) {
+            // no-op, Session Replay is not critical
+            console.error('Error enabling Sentry Replay:', e); // eslint-disable-line no-console
+        }
+    }
+
+    if (environment === 'development') {
+        extraIntegrations.push(new Debug());
+    }
+
+    return config;
+}
+
+export function getSentryTestConfig(transport) {
+    return getSentryConfig(
+        'https://abcdef0123456789abcdef0123456789@o12345.ingest.sentry.io/1234567',
+        'testing',
+        '5.0.0',
+        transport
+    );
+}
 
 export function beforeSend(event, hint) {
     try {

ghost/admin/app/utils/window-proxy.js

@@ -9,5 +9,9 @@ export default {
 
     replaceState(params, title, url) {
         window.history.replaceState(params, title, url);
+    },
+
+    reload() {
+        window.location.reload();
     }
 };

ghost/admin/ember-cli-build.js

@@ -205,6 +205,9 @@ module.exports = function (defaults) {
         },
         autoImport: {
             publicAssetURL,
+            alias: {
+                'sentry-testkit/browser': 'sentry-testkit/dist/browser'
+            },
             webpack: {
                 devtool: 'source-map',
                 resolve: {

ghost/admin/package.json

@@ -147,6 +147,8 @@
     "react-dom": "18.3.1",
     "reframe.js": "4.0.2",
     "semver": "7.6.3",
+    "sentry-testkit": "5.0.9",
+    "sinon-chai": "4.0.0",
     "testem": "3.15.1",
     "tracked-built-ins": "3.3.0",
     "util": "0.12.5",
@@ -212,4 +214,4 @@
       }
     }
   }
-}
+}

ghost/admin/tests/test-helper.js

@@ -6,7 +6,9 @@ import {setApplication} from '@ember/test-helpers';
 
 import chai from 'chai';
 import chaiDom from 'chai-dom';
+import sinonChai from 'sinon-chai';
 chai.use(chaiDom);
+chai.use(sinonChai);
 
 setApplication(Application.create(config.APP));
 

ghost/admin/tests/unit/routes/lexical-editor.new-test.js

@@ -0,0 +1,101 @@
+import * as Sentry from '@sentry/ember';
+import sentryTestkit from 'sentry-testkit/browser';
+import windowProxy from 'ghost-admin/utils/window-proxy';
+import {describe, it} from 'mocha';
+import {expect} from 'chai';
+import {getSentryTestConfig} from 'ghost-admin/utils/sentry';
+import {run} from '@ember/runloop';
+import {setupTest} from 'ember-mocha';
+import {waitUntil} from '@ember/test-helpers';
+
+const {sentryTransport, testkit} = sentryTestkit();
+
+describe('Unit: Route: lexical-editor.new', function () {
+    setupTest();
+
+    let controller;
+    let route;
+    let createRecordStub;
+    let reloadStub;
+
+    before(function () {
+        Sentry.init(getSentryTestConfig(sentryTransport));
+    });
+
+    beforeEach(async function () {
+        // ensure tags don't leak in from earlier (esp. acceptance) tests
+        Sentry.getCurrentHub().getIsolationScope().clear();
+        Sentry.getCurrentScope().clear();
+
+        testkit.reset();
+
+        controller = this.owner.lookup('controller:lexical-editor');
+        route = this.owner.lookup('route:lexical-editor.new');
+        createRecordStub = sinon.stub(route.store, 'createRecord').returns({});
+        reloadStub = sinon.stub(windowProxy, 'reload');
+    });
+
+    afterEach(function () {
+        sinon.restore();
+    });
+
+    describe('didTransition', function () {
+        function callDidTransition() {
+            // we schedule reload to be called in afterRender so we need a runloop
+            run(() => {
+                route.didTransition();
+            });
+        }
+
+        describe('with model.isNew === true', function () {
+            it('does not call reload', function () {
+                const model = {isNew: true};
+                controller.post = model;
+                callDidTransition();
+                expect(reloadStub.called).to.be.false;
+            });
+        });
+
+        describe('with model.isNew === false', function () {
+            it('calls reload', async function () {
+                const model = {isNew: false};
+                controller.post = model;
+                callDidTransition();
+                expect(reloadStub.calledOnce).to.be.true;
+            });
+
+            it('logs to console', async function () {
+                // we attempt to re-create the new post for debug logging purposes
+                // (recreatedPostIsGood = true when the secondary createRecord call results in a good model state)
+                createRecordStub.returns({isNew: true});
+
+                const consoleStub = sinon.stub(console, 'error');
+                const model = {isNew: false};
+                controller.post = model;
+                callDidTransition();
+
+                expect(createRecordStub.calledOnce, 'createRecordStub called').to.be.true;
+                expect(consoleStub).to.have.been.calledWith('New post route transitioned with post.isNew=false', {recreatedPostIsGood: true});
+            });
+
+            it('logs to Sentry', async function () {
+                // we attempt to re-create the new post for debug logging purposes
+                // (recreatedPostIsGood = true when the secondary createRecord call results in a good model state)
+                createRecordStub.returns({isNew: true});
+
+                const model = {isNew: false};
+                controller.post = model;
+                callDidTransition();
+
+                // Sentry reports are delivered async so it's best to wait for them to avoid flaky tests
+                await waitUntil(() => testkit.reports().length > 0);
+
+                expect(testkit.reports()).to.have.length(1);
+                const report = testkit.reports()[0];
+                expect(report.message).to.equal('New post route transitioned with post.isNew=false');
+                expect(report.tags).to.deep.equal({shown_to_user: false, grammarly: false, savePostTask: true});
+                expect(report.extra).to.deep.equal({recreatedPostIsGood: true});
+            });
+        });
+    });
+});

yarn.lock

@@ -11084,7 +11084,7 @@ body-parser@1.20.2:
     type-is "~1.6.18"
     unpipe "1.0.0"
 
-body-parser@1.20.3, body-parser@^1.19.0:
+body-parser@1.20.3, body-parser@^1.19.0, body-parser@^1.20.1:
   version "1.20.3"
   resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.3.tgz#1953431221c6fb5cd63c4b36d53fab0928e548c6"
   integrity sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g==
@@ -28318,6 +28318,14 @@ send@0.19.0:
     range-parser "~1.2.1"
     statuses "2.0.1"
 
+sentry-testkit@5.0.9:
+  version "5.0.9"
+  resolved "https://registry.yarnpkg.com/sentry-testkit/-/sentry-testkit-5.0.9.tgz#28356612116aff9c7f3398913cef63d677ac1b4a"
+  integrity sha512-b9oa3juBM3EXKDU/v5aL4HEC41yGxQZY4pluFN+nvRJLyvHc+JQWPWxxkOPAAr1vGY/4zH+V9NGS+ojIQkc88w==
+  dependencies:
+    body-parser "^1.20.1"
+    express "^4.18.2"
+
 seq-queue@^0.0.5:
   version "0.0.5"
   resolved "https://registry.yarnpkg.com/seq-queue/-/seq-queue-0.0.5.tgz#d56812e1c017a6e4e7c3e3a37a1da6d78dd3c93e"
@@ -28592,6 +28600,11 @@ simple-swizzle@^0.2.2:
   dependencies:
     is-arrayish "^0.3.1"
 
+sinon-chai@4.0.0:
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-4.0.0.tgz#77d59d9f4a833f0d3a88249b4637acc72656fdfa"
+  integrity sha512-cWqO7O2I4XfJDWyWElAQ9D/dtdh5Mo0RHndsfiiYyjWnlPzBJdIvjCVURO4EjyYaC3BjV+ISNXCfTXPXTEIEWA==
+
 sinon@15.2.0:
   version "15.2.0"
   resolved "https://registry.yarnpkg.com/sinon/-/sinon-15.2.0.tgz#5e44d4bc5a9b5d993871137fd3560bebfac27565"