fix: continue startup after module helper failure

js/app.mjs

@@ -39,8 +39,8 @@ spawn("node ./js/systeminformation.js", {
 	stdio: "inherit"
 });
 
-// The next part is here to prevent a major exception when there
-// is no internet connection. This could probable be solved better.
+// Handle uncaught exceptions to keep the process running (e.g. missing network connectivity).
+// TODO: consider more targeted error handling instead of a catch-all
 process.on("uncaughtException", (err) => {
 	// ignore strange exceptions under aarch64 coming from systeminformation:
 	if (!err.stack.includes("node_modules/systeminformation")) {
@@ -155,11 +155,33 @@ class App {
 				}
 			}
 
-			helper.setName(moduleName);
-			helper.setPath(path.resolve(moduleFolder));
-			this.#nodeHelpers.push(helper);
+			try {
+				helper.setName(moduleName);
+				helper.setPath(path.resolve(moduleFolder));
+				await helper.loaded();
+				// Register only after loaded() succeeded so failed helpers are neither started nor stopped.
+				this.#nodeHelpers.push(helper);
+			} catch (error) {
+				Log.error(`Error when initializing node_helper for module ${moduleName}:\n`, error);
+			}
+		}
+	}
 
-			helper.loaded();
+	/**
+	 * Connects a node_helper to the server and starts it. Errors are logged so
+	 * that one failing helper does not prevent the others from starting.
+	 * @param {object} helper The node_helper instance.
+	 * @param {object} app The Express app.
+	 * @param {object} io The Socket.IO server.
+	 * @returns {Promise<void>} Resolved when the helper has started or failed.
+	 */
+	async #startHelper (helper, app, io) {
+		try {
+			helper.setExpressApp(app);
+			helper.setSocketIO(io);
+			await helper.start();
+		} catch (error) {
+			Log.error(`Error when starting node_helper for module ${helper.name}:\n`, error);
 		}
 	}
 
@@ -265,27 +287,8 @@ class App {
 			const { app, io } = await this.#httpServer.open();
 			Log.log("Server started ...");
 
-			const nodePromises = [];
-			for (const nodeHelper of this.#nodeHelpers) {
-				nodeHelper.setExpressApp(app);
-				nodeHelper.setSocketIO(io);
-
-				try {
-					nodePromises.push(nodeHelper.start());
-				} catch (error) {
-					Log.error(`Error when starting node_helper for module ${nodeHelper.name}:`);
-					Log.error(error);
-				}
-			}
-
-			const results = await Promise.allSettled(nodePromises);
-
-			// Log errors that happened during async node_helper startup
-			results.forEach((result) => {
-				if (result.status === "rejected") {
-					Log.error(result.reason);
-				}
-			});
+			const startups = this.#nodeHelpers.map((helper) => this.#startHelper(helper, app, io));
+			await Promise.all(startups);
 
 			Log.log("Sockets connected & modules started ...");
 

tests/configs/isolated_node_helper.js

@@ -0,0 +1,23 @@
+const config = {
+	address: "0.0.0.0",
+	ipWhitelist: [],
+	foreignModulesDir: "tests/mocks",
+	modules: [
+		{
+			module: "esmHelper",
+			position: "bottom_bar"
+		},
+		{
+			module: "calendar",
+			position: "bottom_bar",
+			config: {
+				calendars: []
+			}
+		}
+	]
+};
+
+/*************** DO NOT EDIT THE LINE BELOW ***************/
+if (typeof module !== "undefined") {
+	module.exports = config;
+}

tests/e2e/app_spec.js

@@ -1,25 +1,31 @@
-const NodeHelper = require("../../js/node_helper");
-const helpers = require("./helpers/global-setup");
+import { createRequire } from "node:module";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+const require = createRequire(import.meta.url);
+const Log = require("logger");
+const NodeHelper = require("../../js/node_helper.js");
+const helpers = require("./helpers/global-setup.js");
 
 describe("App restart lifecycle", () => {
 	afterEach(async () => {
 		await helpers.stopApplication(0);
+		vi.restoreAllMocks();
 	});
 
 	it("reloads configuration without retaining stopped helpers or adding signal handlers", async () => {
 		const startHelper = vi.spyOn(NodeHelper.prototype, "start");
 		const stopHelper = vi.spyOn(NodeHelper.prototype, "stop");
 		await helpers.startApplication("tests/configs/esm_helper.js");
-		const app = global.app;
+		const app = globalThis.app;
 		const sigintListeners = process.listenerCount("SIGINT");
 		const sigtermListeners = process.listenerCount("SIGTERM");
 		expect(startHelper).toHaveBeenCalledTimes(1);
-		expect(global.config.modules[0].module).toBe("esmHelper");
+		expect(globalThis.config.modules[0].module).toBe("esmHelper");
 
 		await helpers.startApplication("tests/configs/modules/clock/clock_24hr.js");
-		expect(global.app).toBe(app);
-		expect(global.configuration_file).toBe("tests/configs/modules/clock/clock_24hr.js");
-		expect(global.config.modules[0].module).toBe("clock");
+		expect(globalThis.app).toBe(app);
+		expect(globalThis.configuration_file).toBe("tests/configs/modules/clock/clock_24hr.js");
+		expect(globalThis.config.modules[0].module).toBe("clock");
 		expect(startHelper).toHaveBeenCalledTimes(1);
 		expect(stopHelper).toHaveBeenCalledTimes(1);
 		expect(process.listenerCount("SIGINT")).toBe(sigintListeners);
@@ -29,4 +35,31 @@ describe("App restart lifecycle", () => {
 		await app.stop();
 		expect(stopHelper).toHaveBeenCalledTimes(1);
 	});
+
+	it.each([
+		{ phase: "initialization", hook: "loaded", failure: "throw" },
+		{ phase: "async initialization", hook: "loaded", failure: "reject" },
+		{ phase: "setup", hook: "setExpressApp", failure: "throw" },
+		{ phase: "async startup", hook: "start", failure: "reject" }
+	])("continues startup when a node helper fails during $phase", async ({ phase, hook, failure }) => {
+		const original = NodeHelper.prototype[hook];
+		const helperHook = vi.spyOn(NodeHelper.prototype, hook).mockImplementation(function (...args) {
+			if (this.name === "esmHelper") {
+				const error = new Error(`${phase} failed`);
+				if (failure === "reject") return Promise.reject(error);
+				throw error;
+			}
+			return original.apply(this, args);
+		});
+		const startHelper = hook === "start" ? helperHook : vi.spyOn(NodeHelper.prototype, "start");
+		const logMessage = vi.spyOn(Log, "log");
+		const logError = vi.spyOn(Log, "error");
+
+		const config = await helpers.startApplication("tests/configs/isolated_node_helper.js");
+
+		expect(config.modules).toHaveLength(2);
+		expect(logMessage).toHaveBeenCalledWith("Starting node helper for: calendar");
+		expect(startHelper).toHaveBeenCalledTimes(hook === "start" ? 1 : 0);
+		expect(logError).toHaveBeenCalledWith(expect.stringContaining("esmHelper"), expect.objectContaining({ message: `${phase} failed` }));
+	});
 });