Added error rethrow to update check run as a job

closes https://github.com/TryGhost/Toolbox/issues/525 - When the update check is run as an offloaded job (in a worker thread) the errors that are handled over "logger.error" do not get bubbled up to the job manager to be handled fully. With an optional "rethrowErrors" flag it's not possible to throw errors when the update check is run as a "job" and handle as usual when run in the main thread (through the admin route trigger)
TryGhost · Feb 27, 2023 · 96a64b4 · 96a64b4
1 parent 17a48c7
commit 96a64b4
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 6 deletions.
diff --git a/ghost/core/core/server/run-update-check.js b/ghost/core/core/server/run-update-check.js
@@ -42,7 +42,9 @@ if (parentPort) {
     await settings.init();
     // Finished INIT
 
-    await updateCheck();
+    await updateCheck({
+        rethrowErrors: true
+    });
 
     postParentPortMessage(`Ran update check`);
 

diff --git a/ghost/core/core/server/update-check.js b/ghost/core/core/server/update-check.js
@@ -12,10 +12,11 @@ const UpdateCheckService = require('@tryghost/update-check-service');
 
 /**
  * Initializes and triggers update check
- *
+ * @param {Object} [options]
+ * @param {Boolean} [options.rethrowErrors] - if true, errors will be thrown instead of logged
  * @returns {Promise<any>}
  */
-module.exports = async () => {
+module.exports = async ({rethrowErrors = false} = {}) => {
     const allowedCheckEnvironments = ['development', 'production'];
 
     // CASE: The check will not happen if your NODE_ENV is not in the allowed defined environments.
@@ -51,7 +52,8 @@ module.exports = async () => {
             notificationGroups: config.get('notificationGroups'),
             siteUrl: urlUtils.urlFor('home', true),
             forceUpdate: config.get('updateCheck:forceUpdate'),
-            ghostVersion: ghostVersion.original
+            ghostVersion: ghostVersion.original,
+            rethrowErrors
         },
         request,
         sendEmail: ghostMailer.send.bind(ghostMailer)

diff --git a/ghost/update-check-service/lib/update-check-service.js b/ghost/update-check-service/lib/update-check-service.js
@@ -45,6 +45,7 @@ class UpdateCheckService {
      * @param {string} options.config.checkEndpoint - update check service URL
      * @param {boolean} [options.config.isPrivacyDisabled]
      * @param {string[]} [options.config.notificationGroups] - example values ["migration", "something"]
+     * @param {boolean} [options.config.rethrowErrors] - allows to force throwing errors (useful in worker threads)
      * @param {string} options.config.siteUrl - Ghost instance URL
      * @param {boolean} [options.config.forceUpdate]
      * @param {string} options.config.ghostVersion - Ghost instance version
@@ -88,6 +89,10 @@ class UpdateCheckService {
         err.help = tpl(messages.checkingForUpdatesFailedHelp, {url: 'https://ghost.org/docs/'});
 
         this.logging.error(err);
+
+        if (this.config.rethrowErrors) {
+            throw err;
+        }
     }
 
     /**
@@ -330,7 +335,10 @@ class UpdateCheckService {
                             forceTextContent: true
                         });
                     } catch (err) {
-                        this.logging.err(err);
+                        this.logging.error(err);
+                        if (this.config.rethrowErrors) {
+                            throw err;
+                        }
                     }
                 }
             }

diff --git a/ghost/update-check-service/test/update-check-service.test.js b/ghost/update-check-service/test/update-check-service.test.js
@@ -3,6 +3,7 @@ require('./utils');
 const sinon = require('sinon');
 const moment = require('moment');
 const uuid = require('uuid');
+const assert = require('assert');
 
 const logging = require('@tryghost/logging');
 const UpdateCheckService = require('../lib/update-check-service');
@@ -371,7 +372,8 @@ describe('Update Check', function () {
                     settings: {
                         edit: settingsStub
                     }
-                }
+                },
+                config: {}
             });
 
             updateCheckService.updateCheckError({});
@@ -381,5 +383,30 @@ describe('Update Check', function () {
             logging.error.args[0][0].context.should.equal('Checking for updates failed, your site will continue to function.');
             logging.error.args[0][0].help.should.equal('If you get this error repeatedly, please seek help from https://ghost.org/docs/');
         });
+
+        it('logs and rethrows an error when error with rethrow configuration', function () {
+            const updateCheckService = new UpdateCheckService({
+                api: {
+                    settings: {
+                        edit: settingsStub
+                    }
+                },
+                config: {
+                    rethrowErrors: true
+                }
+            });
+
+            try {
+                updateCheckService.updateCheckError({});
+                assert.fail('should have thrown');
+            } catch (e) {
+                settingsStub.called.should.be.true();
+                logging.error.called.should.be.true();
+                logging.error.args[0][0].context.should.equal('Checking for updates failed, your site will continue to function.');
+                logging.error.args[0][0].help.should.equal('If you get this error repeatedly, please seek help from https://ghost.org/docs/');
+
+                e.context.should.equal('Checking for updates failed, your site will continue to function.');
+            }
+        });
     });
 });