Add error handling to node_helper startup sequence (#2945)

Fixes https://github.com/MichMich/MagicMirror/issues/2944

Also splits the Server js into a constrcutor and an open call to remove
one callback parameter :-)

Co-authored-by: veeck <michael@veeck.de>

CHANGELOG.md

@@ -34,6 +34,7 @@ Special thanks to: @rejas, @sdetweil
 
 - Correctly show apparent temperature in SMHI weather provider
 - Ensure updatenotification module isn't shown when local is _ahead_ of remote
+- Handle node_helper errors during startup (#2944)
 - Possibility to change FontAwesome class in calendar, so icons like `fab fa-facebook-square` works.
 - Fix cors problems with newsfeed articles (as far as possible), allow disabling cors per feed with option `useCorsProxy: false` (#2840)
 

js/app.js

@@ -223,24 +223,35 @@ function App() {
 			}
 
 			loadModules(modules, function () {
-				httpServer = new Server(config, function (app, io) {
+				httpServer = new Server(config);
-					Log.log("Server started ...");
+				const { app, io } = httpServer.open();
+				Log.log("Server started ...");
 
-					const nodePromises = [];
+				const nodePromises = [];
+				for (let nodeHelper of nodeHelpers) {
+					nodeHelper.setExpressApp(app);
+					nodeHelper.setSocketIO(io);
 
-					for (let nodeHelper of nodeHelpers) {
+					try {
-						nodeHelper.setExpressApp(app);
-						nodeHelper.setSocketIO(io);
 						nodePromises.push(nodeHelper.start());
+					} catch (error) {
+						Log.error(`Error when starting node_helper for module ${nodeHelper.name}:`);
+						Log.error(error);
 					}
+				}
 
-					Promise.allSettled(nodePromises).then(() => {
+				Promise.allSettled(nodePromises).then((results) => {
-						Log.log("Sockets connected & modules started ...");
+					// Log errors that happened during async node_helper startup
-
+					results.forEach((result) => {
-						if (typeof callback === "function") {
+						if (result.status === "rejected") {
-							callback(config);
+							Log.error(result.reason);
 						}
 					});
+
+					Log.log("Sockets connected & modules started ...");
+					if (typeof callback === "function") {
+						callback(config);
+					}
 				});
 			});
 		});

js/server.js

@@ -5,7 +5,6 @@
  * MIT Licensed.
  */
 const express = require("express");
-const app = require("express")();
 const path = require("path");
 const ipfilter = require("express-ipfilter").IpFilter;
 const fs = require("fs");
@@ -19,117 +18,119 @@ const Utils = require("./utils.js");
  * Server
  *
  * @param {object} config The MM config
- * @param {Function} callback Function called when done.
  * @class
  */
-function Server(config, callback) {
+function Server(config) {
+	const app = express();
 	const port = process.env.MM_PORT || config.port;
 	const serverSockets = new Set();
-
 	let server = null;
-	if (config.useHttps) {
+
-		const options = {
+	this.open = function () {
-			key: fs.readFileSync(config.httpsPrivateKey),
+		if (config.useHttps) {
-			cert: fs.readFileSync(config.httpsCertificate)
+			const options = {
+				key: fs.readFileSync(config.httpsPrivateKey),
+				cert: fs.readFileSync(config.httpsCertificate)
+			};
+			server = require("https").Server(options, app);
+		} else {
+			server = require("http").Server(app);
+		}
+		const io = require("socket.io")(server, {
+			cors: {
+				origin: /.*$/,
+				credentials: true
+			},
+			allowEIO3: true
+		});
+
+		server.on("connection", (socket) => {
+			serverSockets.add(socket);
+			socket.on("close", () => {
+				serverSockets.delete(socket);
+			});
+		});
+
+		Log.log(`Starting server on port ${port} ... `);
+		server.listen(port, config.address || "localhost");
+
+		if (config.ipWhitelist instanceof Array && config.ipWhitelist.length === 0) {
+			Log.warn(Utils.colors.warn("You're using a full whitelist configuration to allow for all IPs"));
+		}
+
+		app.use(function (req, res, next) {
+			ipfilter(config.ipWhitelist, { mode: config.ipWhitelist.length === 0 ? "deny" : "allow", log: false })(req, res, function (err) {
+				if (err === undefined) {
+					res.header("Access-Control-Allow-Origin", "*");
+					return next();
+				}
+				Log.log(err.message);
+				res.status(403).send("This device is not allowed to access your mirror. <br> Please check your config.js or config.js.sample to change this.");
+			});
+		});
+
+		app.use(helmet(config.httpHeaders));
+		app.use("/js", express.static(__dirname));
+
+		// TODO add tests directory only when running tests?
+		const directories = ["/config", "/css", "/fonts", "/modules", "/vendor", "/translations", "/tests/configs", "/tests/mocks"];
+		for (const directory of directories) {
+			app.use(directory, express.static(path.resolve(global.root_path + directory)));
+		}
+
+		app.get("/cors", async function (req, res) {
+			// example: http://localhost:8080/cors?url=https://google.de
+
+			try {
+				const reg = "^/cors.+url=(.*)";
+				let url = "";
+
+				let match = new RegExp(reg, "g").exec(req.url);
+				if (!match) {
+					url = "invalid url: " + req.url;
+					Log.error(url);
+					res.send(url);
+				} else {
+					url = match[1];
+					Log.log("cors url: " + url);
+					const response = await fetch(url, { headers: { "User-Agent": "Mozilla/5.0 MagicMirror/" + global.version } });
+					const header = response.headers.get("Content-Type");
+					const data = await response.text();
+					if (header) res.set("Content-Type", header);
+					res.send(data);
+				}
+			} catch (error) {
+				Log.error(error);
+				res.send(error);
+			}
+		});
+
+		app.get("/version", function (req, res) {
+			res.send(global.version);
+		});
+
+		app.get("/config", function (req, res) {
+			res.send(config);
+		});
+
+		app.get("/", function (req, res) {
+			let html = fs.readFileSync(path.resolve(`${global.root_path}/index.html`), { encoding: "utf8" });
+			html = html.replace("#VERSION#", global.version);
+
+			let configFile = "config/config.js";
+			if (typeof global.configuration_file !== "undefined") {
+				configFile = global.configuration_file;
+			}
+			html = html.replace("#CONFIG_FILE#", configFile);
+
+			res.send(html);
+		});
+
+		return {
+			app,
+			io
 		};
-		server = require("https").Server(options, app);
+	};
-	} else {
-		server = require("http").Server(app);
-	}
-	const io = require("socket.io")(server, {
-		cors: {
-			origin: /.*$/,
-			credentials: true
-		},
-		allowEIO3: true
-	});
-
-	server.on("connection", (socket) => {
-		serverSockets.add(socket);
-		socket.on("close", () => {
-			serverSockets.delete(socket);
-		});
-	});
-
-	Log.log(`Starting server on port ${port} ... `);
-
-	server.listen(port, config.address || "localhost");
-
-	if (config.ipWhitelist instanceof Array && config.ipWhitelist.length === 0) {
-		Log.warn(Utils.colors.warn("You're using a full whitelist configuration to allow for all IPs"));
-	}
-
-	app.use(function (req, res, next) {
-		ipfilter(config.ipWhitelist, { mode: config.ipWhitelist.length === 0 ? "deny" : "allow", log: false })(req, res, function (err) {
-			if (err === undefined) {
-				res.header("Access-Control-Allow-Origin", "*");
-				return next();
-			}
-			Log.log(err.message);
-			res.status(403).send("This device is not allowed to access your mirror. <br> Please check your config.js or config.js.sample to change this.");
-		});
-	});
-	app.use(helmet(config.httpHeaders));
-
-	app.use("/js", express.static(__dirname));
-
-	// TODO add tests directory only when running tests?
-	const directories = ["/config", "/css", "/fonts", "/modules", "/vendor", "/translations", "/tests/configs", "/tests/mocks"];
-	for (const directory of directories) {
-		app.use(directory, express.static(path.resolve(global.root_path + directory)));
-	}
-
-	app.get("/cors", async function (req, res) {
-		// example: http://localhost:8080/cors?url=https://google.de
-
-		try {
-			const reg = "^/cors.+url=(.*)";
-			let url = "";
-
-			let match = new RegExp(reg, "g").exec(req.url);
-			if (!match) {
-				url = "invalid url: " + req.url;
-				Log.error(url);
-				res.send(url);
-			} else {
-				url = match[1];
-				Log.log("cors url: " + url);
-				const response = await fetch(url, { headers: { "User-Agent": "Mozilla/5.0 MagicMirror/" + global.version } });
-				const header = response.headers.get("Content-Type");
-				const data = await response.text();
-				if (header) res.set("Content-Type", header);
-				res.send(data);
-			}
-		} catch (error) {
-			Log.error(error);
-			res.send(error);
-		}
-	});
-
-	app.get("/version", function (req, res) {
-		res.send(global.version);
-	});
-
-	app.get("/config", function (req, res) {
-		res.send(config);
-	});
-
-	app.get("/", function (req, res) {
-		let html = fs.readFileSync(path.resolve(`${global.root_path}/index.html`), { encoding: "utf8" });
-		html = html.replace("#VERSION#", global.version);
-
-		let configFile = "config/config.js";
-		if (typeof global.configuration_file !== "undefined") {
-			configFile = global.configuration_file;
-		}
-		html = html.replace("#CONFIG_FILE#", configFile);
-
-		res.send(html);
-	});
-
-	if (typeof callback === "function") {
-		callback(app, io);
-	}
 
 	this.close = function () {
 		for (const socket of serverSockets.values()) {