From fe0b915a5d40ac4af4b007c4707ebf1466bc3fe7 Mon Sep 17 00:00:00 2001 From: Veeck Date: Wed, 22 Feb 2023 18:58:00 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + js/app.js | 130 ++++++++++++++++++------------ js/electron.js | 13 ++- serveronly/index.js | 2 +- tests/e2e/helpers/global-setup.js | 14 ++-- 5 files changed, 90 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 413afd54..bc999007 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/js/app.js b/js/app.js index 943d9c61..8a016333 100644 --- a/js/app.js +++ b/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} 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,59 +261,55 @@ 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} the config used */ - this.start = function (callback) { - loadConfig().then((c) => { - config = c; + this.start = async function () { + config = await loadConfig(); - Log.setLogLevel(config.logLevel); + Log.setLogLevel(config.logLevel); - let modules = []; + let modules = []; - for (const module of config.modules) { - if (!modules.includes(module.module) && !module.disabled) { - modules.push(module.module); + for (const module of config.modules) { + if (!modules.includes(module.module) && !module.disabled) { + modules.push(module.module); + } + } + + loadModules(modules, async function () { + httpServer = new Server(config); + const { app, io } = await httpServer.open(); + Log.log("Server started ..."); + + const nodePromises = []; + for (let nodeHelper of 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); } } - loadModules(modules, async function () { - httpServer = new Server(config); - const { app, io } = await httpServer.open(); - Log.log("Server started ..."); + const results = await Promise.allSettled(nodePromises); - const nodePromises = []; - for (let nodeHelper of 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); - } + // Log errors that happened during async node_helper startup + results.forEach((result) => { + if (result.status === "rejected") { + Log.error(result.reason); } - - Promise.allSettled(nodePromises).then((results) => { - // Log errors that happened during async node_helper startup - results.forEach((result) => { - if (result.status === "rejected") { - Log.error(result.reason); - } - }); - - Log.log("Sockets connected & modules started ..."); - }); }); - if (typeof callback === "function") { - callback(config); - } + Log.log("Sockets connected & modules started ..."); }); + + 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) { - if (typeof nodeHelper.stop === "function") { - nodeHelper.stop(); + this.stop = async function () { + const nodePromises = []; + for (let nodeHelper of nodeHelpers) { + try { + if (typeof nodeHelper.stop === "function") { + 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); }); } diff --git a/js/electron.js b/js/electron.js index 9e4d3f5e..13247cea 100644 --- a/js/electron.js +++ b/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)); } diff --git a/serveronly/index.js b/serveronly/index.js index 00d6b64b..42cfd049 100644 --- a/serveronly/index.js +++ b/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); diff --git a/tests/e2e/helpers/global-setup.js b/tests/e2e/helpers/global-setup.js index 582ab3ec..93612847 100644 --- a/tests/e2e/helpers/global-setup.js +++ b/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(); } - return Promise.resolve(); + await global.app.stop(); + delete global.app; }; exports.getDocument = () => {