diff --git a/public/src/utils.js b/public/src/utils.js index e3b0347de0..2f427fd49d 100644 --- a/public/src/utils.js +++ b/public/src/utils.js @@ -4,8 +4,7 @@ if (typeof module === 'object' && module.exports) { var winston = require('winston'); - - module.exports = factory(require('xregexp')); + module.exports = factory(require('xregexp'), winston); module.exports.walk = function (dir, done) { // DEPRECATED var file = require('../../src/file'); @@ -14,7 +13,7 @@ }; process.profile = function (operation, start) { - console.log('%s took %d milliseconds', operation, process.elapsedTimeSince(start)); + winston.log('%s took %d milliseconds', operation, process.elapsedTimeSince(start)); }; process.elapsedTimeSince = function (start) { @@ -22,9 +21,10 @@ return (diff[0] * 1e3) + (diff[1] / 1e6); }; } else { - window.utils = factory(window.XRegExp); + window.utils = factory(window.XRegExp, console); } -}(function (XRegExp) { + // eslint-disable-next-line +}(function (XRegExp, console) { var freeze = Object.freeze || function (obj) { return obj; }; // add default escape function for escaping HTML entities @@ -469,6 +469,11 @@ return utils.extensionMimeTypeMap[extension] || '*'; }, + isPromise: function (object) { + // https://stackoverflow.com/questions/27746304/how-do-i-tell-if-an-object-is-a-promise#comment97339131_27746324 + return object && typeof object.then === 'function'; + }, + isRelativeUrl: function (url) { var firstChar = String(url || '').charAt(0); return (firstChar === '.' || firstChar === '/'); diff --git a/src/plugins/hooks.js b/src/plugins/hooks.js index 5c582dbad2..36c4d703a2 100644 --- a/src/plugins/hooks.js +++ b/src/plugins/hooks.js @@ -1,7 +1,8 @@ 'use strict'; -var winston = require('winston'); -var async = require('async'); +const winston = require('winston'); +const async = require('async'); +const utils = require('../utils'); module.exports = function (Plugins) { Plugins.deprecatedHooks = { @@ -132,8 +133,13 @@ module.exports = function (Plugins) { } return next(null, params); } - - hookObj.method(params, next); + const returned = hookObj.method(params, next); + if (utils.isPromise(returned)) { + returned.then( + payload => setImmediate(next, null, payload), + err => setImmediate(next, err) + ); + } }, callback); } @@ -160,26 +166,35 @@ module.exports = function (Plugins) { } async.each(hookList, function (hookObj, next) { if (typeof hookObj.method === 'function') { - var timedOut = false; - - var timeoutId = setTimeout(function () { + let timedOut = false; + const timeoutId = setTimeout(function () { winston.warn('[plugins] Callback timed out, hook \'' + hook + '\' in plugin \'' + hookObj.id + '\''); timedOut = true; next(); }, 5000); - try { - hookObj.method(params, function () { - clearTimeout(timeoutId); - if (!timedOut) { - next.apply(null, arguments); - } - }); - } catch (err) { + const onError = (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); } } else { next(); diff --git a/test/plugins.js b/test/plugins.js index a717f395c3..e01d74ed18 100644 --- a/test/plugins.js +++ b/test/plugins.js @@ -47,6 +47,42 @@ describe('Plugins', function () { }); }); + it('should register and fire a filter hook having 2 methods, one returning a promise and the other calling the callback', function (done) { + function method1(data, callback) { + data.foo += 1; + callback(null, data); + } + function method2(data) { + return new Promise(function (resolve) { + data.foo += 5; + resolve(data); + }); + } + + plugins.registerHook('test-plugin', { hook: 'filter:test.hook2', method: method1 }); + plugins.registerHook('test-plugin', { hook: 'filter:test.hook2', method: method2 }); + + plugins.fireHook('filter:test.hook2', { foo: 1 }, function (err, data) { + assert.ifError(err); + assert.equal(data.foo, 7); + done(); + }); + }); + + it('should register and fire a filter hook that returns a promise that gets rejected', function (done) { + function method(data) { + return new Promise(function (resolve, reject) { + data.foo += 5; + reject(new Error('nope')); + }); + } + plugins.registerHook('test-plugin', { hook: 'filter:test.hook3', method: method }); + plugins.fireHook('filter:test.hook3', { foo: 1 }, function (err) { + assert(err); + done(); + }); + }); + it('should register and fire an action hook', function (done) { function actionMethod(data) { assert.equal(data.bar, 'test'); @@ -70,6 +106,48 @@ describe('Plugins', function () { }); }); + it('should register and fire a static hook returning a promise', function (done) { + function method(data) { + assert.equal(data.bar, 'test'); + return new Promise((resolve) => { + resolve(); + }); + } + plugins.registerHook('test-plugin', { hook: 'static:test.hook', method: method }); + plugins.fireHook('static:test.hook', { bar: 'test' }, function (err) { + assert.ifError(err); + done(); + }); + }); + + it('should register and fire a static hook returning a promise that gets rejected with a warning only', function (done) { + function method(data) { + assert.equal(data.bar, 'test'); + return new Promise(function (resolve, reject) { + reject(new Error('just because')); + }); + } + plugins.registerHook('test-plugin', { hook: 'static:test.hook', method: method }); + plugins.fireHook('static:test.hook', { bar: 'test' }, function (err) { + assert.ifError(err); + done(); + }); + }); + + it('should register and timeout a static hook returning a promise but takes too long', function (done) { + function method(data) { + assert.equal(data.bar, 'test'); + return new Promise(function (resolve) { + setTimeout(resolve, 6000); + }); + } + plugins.registerHook('test-plugin', { hook: 'static:test.hook', method: method }); + plugins.fireHook('static:test.hook', { bar: 'test' }, function (err) { + assert.ifError(err); + done(); + }); + }); + it('should get plugin data from nbbpm', function (done) { plugins.get('nodebb-plugin-markdown', function (err, data) { assert.ifError(err);