From e7bd038db13b41bdd6ec13c232306b58c9274c4a Mon Sep 17 00:00:00 2001
From: Julian Lam <julian@nodebb.org>
Date: Wed, 27 Jan 2021 10:51:38 -0500
Subject: [PATCH] refactor(hooks): better error handling

Hook listeners can both throw and reject, and it will be handled appropriately (e.g. filters won't fail)
---
 public/src/modules/hooks.js | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/public/src/modules/hooks.js b/public/src/modules/hooks.js
index fa06d46a97..422e85667e 100644
--- a/public/src/modules/hooks.js
+++ b/public/src/modules/hooks.js
@@ -5,17 +5,30 @@ define('hooks', [], () => {
 		loaded: {},
 	};
 
-	Hooks.register = (hookName, listener) => {
+	Hooks.register = (hookName, method) => {
 		Hooks.loaded[hookName] = Hooks.loaded[hookName] || new Set();
-		Hooks.loaded[hookName].add(listener);
+		Hooks.loaded[hookName].add(method);
 	};
 	Hooks.on = Hooks.register;
 
 	Hooks.hasListeners = hookName => Hooks.loaded[hookName] && Hooks.loaded[hookName].length > 0;
 
+	const _onHookError = (e, listener, data) => {
+		console.warn(`[hooks] Exception encountered in ${listener.name ? listener.name : 'anonymous function'}, stack trace follows.`);
+		console.error(e);
+		return Promise.resolve(data);
+	};
+
 	const _fireFilterHook = (hookName, data) => {
 		const listeners = Array.from(Hooks.loaded[hookName]);
-		return listeners.reduce((promise, listener) => promise.then(data => listener(data)), Promise.resolve(data));
+		return listeners.reduce((promise, listener) => promise.then((data) => {
+			try {
+				const result = listener(data);
+				return utils.isPromise(result) ? result.then(data => Promise.resolve(data)).catch(e => _onHookError(e, listener, data)) : result;
+			} catch (e) {
+				return _onHookError(e, listener, data);
+			}
+		}), Promise.resolve(data));
 	};
 
 	const _fireActionHook = (hookName, data) => {
@@ -27,7 +40,13 @@ define('hooks', [], () => {
 
 	const _fireStaticHook = (hookName, data) => {
 		const listeners = Array.from(Hooks.loaded[hookName]);
-		return Promise.allSettled(listeners.map(listener => listener(data))).then(() => Promise.resolve(data));
+		return Promise.allSettled(listeners.map((listener) => {
+			try {
+				return listener(data);
+			} catch (e) {
+				return _onHookError(e, listener);
+			}
+		})).then(() => Promise.resolve(data));
 	};
 
 	Hooks.fire = (hookName, data) => {