Invoke plugin hooks via common function

Use a common function to iterate through and invoke the given hook in
every plugin, and ignore errors thrown by plugins from these hooks.

lib/plugin.js

@@ -334,6 +334,30 @@ function attachPluginMessages(link, pluginInfo, plugin) {
 	}
 }
 
+/**
+ * Invokes the given hook on all plugins
+ *
+ * @param {Map<string, Object} plugins -
+ *     Mapping of plugin names to plugins to invoke the hook on.
+ * @param {string} hook - Name of hook to invoke.
+ * @param {...*} args - Arguments to pass on to the hook.
+ * @returns {Array} Non-empty return values from the hooks.
+ */
+async function invokeHook(plugins, hook, ...args) {
+	let results = [];
+	for (let [name, plugin] of plugins) {
+		try {
+			let result = await plugin[hook](...args);
+			if (result !== undefined) {
+				results.push(result);
+			}
+		} catch (err) {
+			console.error(`Ignoring error from plugin ${name} in ${hook}:`, err);
+		}
+	}
+	return results;
+}
+
 
 module.exports = {
 	BaseInstancePlugin,
@@ -341,4 +365,5 @@ module.exports = {
 
 	loadPluginInfos,
 	attachPluginMessages,
+	invokeHook,
 };

master.js

@@ -128,12 +128,10 @@ async function getMetrics(req, res, next) {
 	endpointHitCounter.labels(req.route.path).inc();
 
 	let results = []
-	for (let masterPlugin of masterPlugins.values()) {
-		let pluginResults = await masterPlugin.onMetrics();
-		if (pluginResults !== undefined) {
-			for await (let result of pluginResults) {
-				results.push(result)
-			}
+	let pluginResults = await plugin.invokeHook(masterPlugins, "onMetrics");
+	for (let metricIterator of pluginResults) {
+		for await (let metric of metricIterator) {
+			results.push(metric)
 		}
 	}
 
@@ -291,11 +289,7 @@ async function shutdown() {
 		await saveMap(masterConfig.get('master.database_directory'), "slaves.json", db.slaves);
 		await saveInstances(masterConfig.get('master.database_directory'), "instances.json", db.instances);
 
-		for (let [pluginName, masterPlugin] of masterPlugins) {
-			let startTime = Date.now();
-			await masterPlugin.onShutdown();
-			console.log(`Plugin ${pluginName} shutdown in ${Date.now() - startTime}ms`);
-		}
+		await plugin.invokeHook(masterPlugins, "onShutdown");
 
 		stopAcceptingNewSessions = true;
 
@@ -427,9 +421,7 @@ class BaseConnection extends link.Link {
 	}
 
 	async prepareDisconnectRequestHandler(message, request) {
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onPrepareSlaveDisconnect(this);
-		}
+		await plugin.invokeHook(masterPlugins, "onPrepareSlaveDisconnect", this);
 		this.connector.setClosing();
 		return await super.prepareDisconnectRequestHandler(message, request);
 	}
@@ -652,9 +644,7 @@ class SlaveConnection extends BaseConnection {
 		let prev = instance.status;
 		instance.status = "initialized";
 		console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Initialized`)
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onInstanceStatusChanged(instance, prev);
-		}
+		await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 	}
 
 	async instanceStartedEventHandler(message, event) {
@@ -662,9 +652,7 @@ class SlaveConnection extends BaseConnection {
 		let prev = instance.status;
 		instance.status = "running";
 		console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Started`)
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onInstanceStatusChanged(instance, prev);
-		}
+		await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 	}
 
 	async instanceStoppedEventHandler(message, event) {
@@ -672,9 +660,7 @@ class SlaveConnection extends BaseConnection {
 		let prev = instance.status;
 		instance.status = "stopped";
 		console.log(`Clusterio | Instance ${instance.config.get("instance.name")} Stopped`)
-		for (let masterPlugin of masterPlugins.values()) {
-			await masterPlugin.onInstanceStatusChanged(instance, prev);
-		}
+		await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 	}
 
 	async updateInstancesEventHandler(message) {
@@ -700,9 +686,7 @@ class SlaveConnection extends BaseConnection {
 					let prev = masterInstance.status;
 					masterInstance.status = instance.status;
 					if (prev !== undefined) {
-						for (let masterPlugin of masterPlugins.values()) {
-							await masterPlugin.onInstanceStatusChanged(instance, prev);
-						}
+						await plugin.invokeHook(masterPlugins, "onInstanceStatusChanged", instance, prev);
 					}
 				}
 				continue;

slave.js

@@ -89,11 +89,7 @@ class Instance extends link.Link{
 		this.server.on('output', (output) => {
 			link.messages.instanceOutput.send(this, { instance_id: this.config.get("instance.id"), output })
 
-			for (let [name, plugin] of this.plugins) {
-				plugin.onOutput(output).catch(err => {
-					console.error(`Plugin ${name} raised error in onOutput:`, err);
-				});
-			}
+			plugin.invokeHook(this.plugins, "onOutput", output);
 		});
 
 		this.server.on("error", err => {
@@ -337,9 +333,7 @@ class Instance extends link.Link{
 		await this.server.disableAchievements()
 		await this.updateInstanceData();
 
-		for (let pluginInstance of this.plugins.values()) {
-			await pluginInstance.onStart();
-		}
+		await plugin.invokeHook(this.plugins, "onStart");
 
 		this._running = true;
 		link.messages.instanceStarted.send(this, { instance_id: this.config.get("instance.id") });
@@ -362,35 +356,26 @@ class Instance extends link.Link{
 
 		// XXX this needs more thought to it
 		if (this.server._state === "running") {
-			for (let pluginInstance of this.plugins.values()) {
-				await pluginInstance.onStop();
-			}
-
+			await plugin.invokeHook(this.plugins, "onStop");
 			await this.server.stop();
 		}
 	}
 
 	async masterConnectionEventEventHandler(message) {
-		for (let pluginInstance of this.plugins.values()) {
-			pluginInstance.onMasterConnectionEvent(message.data.event);
-		}
+		await plugin.invokeHook(this.plugins, "onMasterConnectionEvent", message.data.event);
 	}
 
 	async prepareMasterDisconnectRequestHandler() {
-		for (let pluginInstance of this.plugins.values()) {
-			await pluginInstance.onPrepareMasterDisconnect();
-		}
+		await plugin.invokeHook(this.plugins, "onPrepareMasterDisconnect");
 	}
 
 	async getMetricsRequestHandler() {
 		let results = []
 		if (this._running) {
-			for (let pluginInstance of this.plugins.values()) {
-				let pluginResults = await pluginInstance.onMetrics();
-				if (pluginResults !== undefined) {
-					for await (let result of pluginResults) {
-						results.push(prometheus.serializeResult(result))
-					}
+			let pluginResults = await plugin.invokeHook(this.plugins, "onMetrics");
+			for (let metricIterator of pluginResults) {
+				for await (let metric of metricIterator) {
+					results.push(prometheus.serializeResult(metric))
 				}
 			}
 		}

test/lib/plugin.js

@@ -119,4 +119,31 @@ describe("lib/plugin", function() {
 			);
 		});
 	});
+
+	describe("invokeHook()", function() {
+		let betaTestCalled = false;
+		let plugins = new Map([
+			["alpha", {
+				test: async function() { },
+				pass: async function(arg) { return arg; },
+				error: async function() { throw new Error("Test"); },
+			}],
+			["beta", {
+				test: async function() { betaTestCalled = true; },
+				pass: async function() { },
+				error: async function() { },
+			}],
+		]);
+		it("should invoke the hook on the plugin", async function() {
+			await plugin.invokeHook(plugins, "test");
+			assert(betaTestCalled, "Hook was not called");
+		});
+		it("should pass and return args", async function() {
+			let result = await plugin.invokeHook(plugins, "pass", 1234);
+			assert.deepEqual(result, [1234]);
+		});
+		it("should ignore errors", async function() {
+			await plugin.invokeHook(plugins, "error");
+		});
+	});
 });