diff --git a/src/plugins/hooks.js b/src/plugins/hooks.js index 3ad98a809f..7f9e303857 100644 --- a/src/plugins/hooks.js +++ b/src/plugins/hooks.js @@ -148,40 +148,40 @@ module.exports = function (Plugins) { if (!Array.isArray(hookList) || !hookList.length) { return; } + // don't bubble errors from these hooks, so bad plugins don't stop startup + const noErrorHooks = ['static:app.load', 'static:assets.prepare', 'static:app.preload']; await async.each(hookList, function (hookObj, next) { - if (typeof hookObj.method === 'function') { - let timedOut = false; - const timeoutId = setTimeout(function () { - winston.warn('[plugins] Callback timed out, hook \'' + hook + '\' in plugin \'' + hookObj.id + '\''); - timedOut = true; - next(); - }, 5000); - - const onError = (err) => { + if (typeof hookObj.method !== 'function') { + return next(); + } + + let timedOut = false; + const timeoutId = setTimeout(function () { + winston.warn('[plugins] Callback timed out, hook \'' + hook + '\' in plugin \'' + hookObj.id + '\''); + timedOut = true; + next(); + }, 5000); + + const callback = (err) => { + clearTimeout(timeoutId); + if (err) { winston.error('[plugins] Error executing \'' + hook + '\' in plugin \'' + hookObj.id + '\''); - winston.error(err); - clearTimeout(timeoutId); - next(); - }; - const callback = (...args) => { - clearTimeout(timeoutId); - if (!timedOut) { - next(...args); - } - }; - try { - const returned = hookObj.method(params, callback); - if (utils.isPromise(returned)) { - returned.then( - payload => setImmediate(callback, null, payload), - err => setImmediate(onError, err) - ); - } - } catch (err) { - onError(err); + winston.error(err.stack); } - } else { - next(); + if (!timedOut) { + next(noErrorHooks.includes(hook) ? null : err); + } + }; + try { + const returned = hookObj.method(params, callback); + if (utils.isPromise(returned)) { + returned.then( + payload => setImmediate(callback, null, payload), + err => setImmediate(callback, err) + ); + } + } catch (err) { + callback(err); } }); } diff --git a/test/plugins.js b/test/plugins.js index e01d74ed18..a029d06c06 100644 --- a/test/plugins.js +++ b/test/plugins.js @@ -120,7 +120,7 @@ describe('Plugins', function () { }); }); - it('should register and fire a static hook returning a promise that gets rejected with a warning only', function (done) { + it('should register and fire a static hook returning a promise that gets rejected with a error', function (done) { function method(data) { assert.equal(data.bar, 'test'); return new Promise(function (resolve, reject) { @@ -129,7 +129,8 @@ describe('Plugins', function () { } plugins.registerHook('test-plugin', { hook: 'static:test.hook', method: method }); plugins.fireHook('static:test.hook', { bar: 'test' }, function (err) { - assert.ifError(err); + assert.strictEqual(err.message, 'just because'); + plugins.unregisterHook('test-plugin', 'static:test.hook', method); done(); }); }); @@ -144,6 +145,7 @@ describe('Plugins', function () { plugins.registerHook('test-plugin', { hook: 'static:test.hook', method: method }); plugins.fireHook('static:test.hook', { bar: 'test' }, function (err) { assert.ifError(err); + plugins.unregisterHook('test-plugin', 'static:test.hook', method); done(); }); });