feat: hooks can now return promise or call callbacks

* hooks can now return promise or call callbacks, either way works

* cleanups

* moar cleanups and fix callback 1st null arg

* rm unnessesary spread

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 === '/');

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();

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);