Convert app-start/-stop callbacks to async/await (#3035)

supersedes https://github.com/MichMich/MagicMirror/pull/3027 and just
touches the start/stop calls.

---------

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

CHANGELOG.md

@@ -38,6 +38,7 @@ _This release is scheduled to be released on 2023-04-01._
 - Cleanup jest coverage for patches
 - Update `stylelint` dependencies, switch to `stylelint-config-standard` and handle `stylelint` issues
 - Convert translator callbacks to async/await
+- Convert app-start/-stop callbacks to async/awaits
 
 ### Fixed
 

js/app.js

@@ -53,6 +53,9 @@ function App() {
 
 	/**
 	 * Loads the config file. Combines it with the defaults and returns the config
+	 *
+	 * @async
+	 * @returns {Promise<object>} the loaded config or the defaults if something goes wrong
 	 */
 	async function loadConfig() {
 		Log.log("Loading config ...");
@@ -115,8 +118,7 @@ function App() {
 			fs.accessSync(configFilename, fs.F_OK);
 			const c = require(configFilename);
 			checkDeprecatedOptions(c);
-			const config = Object.assign(defaults, c);
-			return config;
+			return Object.assign(defaults, c);
 		} catch (e) {
 			if (e.code === "ENOENT") {
 				Log.error(Utils.colors.error("WARNING! Could not find config file. Please create one. Starting with default configuration."));
@@ -125,8 +127,9 @@ function App() {
 			} else {
 				Log.error(Utils.colors.error(`WARNING! Could not load config file. Starting with default configuration. Error found: ${e}`));
 			}
-			return defaults;
 		}
+
+		return defaults;
 	}
 
 	/**
@@ -258,14 +261,13 @@ function App() {
 	/**
 	 * Start the core app.
 	 *
-	 * It loads the config, then it loads all modules. When it's done it
-	 * executes the callback with the config as argument.
+	 * It loads the config, then it loads all modules.
 	 *
-	 * @param {Function} callback Function to be called after start
+	 * @async
+	 * @returns {Promise<object>} the config used
 	 */
-	this.start = function (callback) {
-		loadConfig().then((c) => {
-			config = c;
+	this.start = async function () {
+		config = await loadConfig();
 
 		Log.setLogLevel(config.logLevel);
 
@@ -295,7 +297,8 @@ function App() {
 				}
 			}
 
-				Promise.allSettled(nodePromises).then((results) => {
+			const results = await Promise.allSettled(nodePromises);
+
 			// Log errors that happened during async node_helper startup
 			results.forEach((result) => {
 				if (result.status === "rejected") {
@@ -305,12 +308,8 @@ function App() {
 
 			Log.log("Sockets connected & modules started ...");
 		});
-			});
 
-			if (typeof callback === "function") {
-				callback(config);
-			}
-		});
+		return config;
 	};
 
 	/**
@@ -319,15 +318,40 @@ function App() {
 	 *
 	 * Added to fix #1056
 	 *
-	 * @param {Function} callback Function to be called after the app has stopped
+	 * @returns {Promise} A promise that is resolved when all node_helpers and
+	 * the http server has been closed
 	 */
-	this.stop = function (callback) {
-		for (const nodeHelper of nodeHelpers) {
+	this.stop = async function () {
+		const nodePromises = [];
+		for (let nodeHelper of nodeHelpers) {
+			try {
 				if (typeof nodeHelper.stop === "function") {
-				nodeHelper.stop();
+					nodePromises.push(nodeHelper.stop());
+				}
+			} catch (error) {
+				Log.error(`Error when stopping node_helper for module ${nodeHelper.name}:`);
+				console.error(error);
 			}
 		}
-		httpServer.close().then(callback);
+
+		const results = await Promise.allSettled(nodePromises);
+
+		// Log errors that happened during async node_helper stopping
+		results.forEach((result) => {
+			if (result.status === "rejected") {
+				Log.error(result.reason);
+			}
+		});
+
+		Log.log("Node_helpers stopped ...");
+
+		// To be able to stop the app even if it hasn't been started (when
+		// running with Electron against another server)
+		if (!httpServer) {
+			return Promise.resolve();
+		}
+
+		return httpServer.close();
 	};
 
 	/**
@@ -337,12 +361,12 @@ function App() {
 	 * Note: this is only used if running `server-only`. Otherwise
 	 * this.stop() is called by app.on("before-quit"... in `electron.js`
 	 */
-	process.on("SIGINT", () => {
+	process.on("SIGINT", async () => {
 		Log.log("[SIGINT] Received. Shutting down server...");
 		setTimeout(() => {
 			process.exit(0);
 		}, 3000); // Force quit after 3 seconds
-		this.stop();
+		await this.stop();
 		process.exit(0);
 	});
 
@@ -350,12 +374,12 @@ function App() {
 	 * Listen to SIGTERM signals so we can stop everything when we
 	 * are asked to stop by the OS.
 	 */
-	process.on("SIGTERM", () => {
+	process.on("SIGTERM", async () => {
 		Log.log("[SIGTERM] Received. Shutting down server...");
 		setTimeout(() => {
 			process.exit(0);
 		}, 3000); // Force quit after 3 seconds
-		this.stop();
+		await this.stop();
 		process.exit(0);
 	});
 }

js/electron.js

@@ -157,18 +157,19 @@ app.on("activate", function () {
  * Note: this is only used if running Electron. Otherwise
  * core.stop() is called by process.on("SIGINT"... in `app.js`
  */
-app.on("before-quit", (event) => {
+app.on("before-quit", async (event) => {
 	Log.log("Shutting down server...");
 	event.preventDefault();
 	setTimeout(() => {
 		process.exit(0);
 	}, 3000); // Force-quit after 3 seconds.
-	core.stop();
+	await core.stop();
 	process.exit(0);
 });
 
-/* handle errors from self signed certificates */
-
+/**
+ * Handle errors from self-signed certificates
+ */
 app.on("certificate-error", (event, webContents, url, error, certificate, callback) => {
 	event.preventDefault();
 	callback(true);
@@ -177,7 +178,5 @@ app.on("certificate-error", (event, webContents, url, error, certificate, callba
 // Start the core application if server is run on localhost
 // This starts all node helpers and starts the webserver.
 if (["localhost", "127.0.0.1", "::1", "::ffff:127.0.0.1", undefined].includes(config.address)) {
-	core.start(function (c) {
-		config = c;
-	});
+	core.start().then((c) => (config = c));
 }

serveronly/index.js

@@ -1,7 +1,7 @@
 const app = require("../js/app.js");
 const Log = require("logger");
 
-app.start((config) => {
+app.start().then((config) => {
 	const bindAddress = config.address ? config.address : "localhost";
 	const httpType = config.useHttps ? "https" : "http";
 	Log.log("\nReady to go! Please point your browser to: " + httpType + "://" + bindAddress + ":" + config.port);

tests/e2e/helpers/global-setup.js

@@ -15,19 +15,15 @@ exports.startApplication = async (configFilename, exec) => {
 	if (exec) exec;
 	global.app = require("app.js");
 
-	return new Promise((resolve) => {
-		global.app.start(resolve);
-	});
+	return global.app.start();
 };
 
 exports.stopApplication = async () => {
-	if (global.app) {
-		return new Promise((resolve) => {
-			global.app.stop(resolve);
-			delete global.app;
-		});
-	}
+	if (!global.app) {
 		return Promise.resolve();
+	}
+	await global.app.stop();
+	delete global.app;
 };
 
 exports.getDocument = () => {