diff --git a/js/app.mjs b/js/app.mjs index 28484db172..355b767c42 100644 --- a/js/app.mjs +++ b/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} 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 ..."); diff --git a/tests/configs/isolated_node_helper.js b/tests/configs/isolated_node_helper.js new file mode 100644 index 0000000000..65a82e0c4d --- /dev/null +++ b/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; +} diff --git a/tests/e2e/app_spec.js b/tests/e2e/app_spec.js index 7981453eeb..3de28f0619 100644 --- a/tests/e2e/app_spec.js +++ b/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` })); + }); });