From f12a513796c9e9aba0bf07776765686978b7ad0f Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 12 Mar 2021 18:30:16 +0100 Subject: [PATCH 1/3] First save config, then create new service client --- nodecg-io-core/extension/instanceManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nodecg-io-core/extension/instanceManager.ts b/nodecg-io-core/extension/instanceManager.ts index 99e3de0bc..702fbe169 100644 --- a/nodecg-io-core/extension/instanceManager.ts +++ b/nodecg-io-core/extension/instanceManager.ts @@ -172,13 +172,13 @@ export class InstanceManager extends EventEmitter { } } - // All checks passed. Set config. + // All checks passed. Set config and save it. inst.config = config; + this.emit("change"); // Update client of this instance using the new config. const updateResult = await this.updateInstanceClient(inst, instanceName, service.result); - this.emit("change"); return updateResult; } From ef9fc4cbd5de0a3b261307a2efa3b4a0a19056da Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 12 Mar 2021 18:34:09 +0100 Subject: [PATCH 2/3] Revert "Temporarily disable save on exit" This reverts commit a04b9ff874ee00087661714704d014240d35bbe4. --- nodecg-io-core/extension/index.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/nodecg-io-core/extension/index.ts b/nodecg-io-core/extension/index.ts index 624abaa25..6a7e7f162 100644 --- a/nodecg-io-core/extension/index.ts +++ b/nodecg-io-core/extension/index.ts @@ -32,7 +32,7 @@ module.exports = (nodecg: NodeCG): NodeCGIOCore => { persistenceManager, ).registerMessageHandlers(); - registerExitHandlers(nodecg, bundleManager, instanceManager, serviceManager); + registerExitHandlers(nodecg, bundleManager, instanceManager, serviceManager, persistenceManager); // We use a extra object instead of returning a object containing all the managers and so on, because // any loaded bundle would be able to call any (public or private) of the managers which is not intended. @@ -62,7 +62,12 @@ function onExit( bundleManager: BundleManager, instanceManager: InstanceManager, serviceManager: ServiceManager, + persistenceManager: PersistenceManager, ): void { + // Save everything + // This is especially important if some services update some configs (e.g. updated tokens) and they haven't been saved yet. + persistenceManager.save(); + // Unset all service instances in all bundles const bundles = bundleManager.getBundleDependencies(); for (const bundleName in bundles) { @@ -99,9 +104,10 @@ function registerExitHandlers( bundleManager: BundleManager, instanceManager: InstanceManager, serviceManager: ServiceManager, + persistenceManager: PersistenceManager, ): void { const handler = () => { - onExit(nodecg, bundleManager, instanceManager, serviceManager); + onExit(nodecg, bundleManager, instanceManager, serviceManager, persistenceManager); }; // Normal exit From b29d36bd85bd2ac1664e40eea402581ead29714e Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 12 Mar 2021 18:55:21 +0100 Subject: [PATCH 3/3] Rewrite updateInstanceConfig to instantly set the config if no validation is needed. --- nodecg-io-core/extension/instanceManager.ts | 33 ++++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/nodecg-io-core/extension/instanceManager.ts b/nodecg-io-core/extension/instanceManager.ts index 702fbe169..77d52ddf8 100644 --- a/nodecg-io-core/extension/instanceManager.ts +++ b/nodecg-io-core/extension/instanceManager.ts @@ -140,19 +140,30 @@ export class InstanceManager extends EventEmitter { * Should only be false if it has been validated at a previous point in time, e.g. loading after startup. * @return void if everything went fine and a string describing the issue if something went wrong. */ - async updateInstanceConfig(instanceName: string, config: unknown, validation = true): Promise> { + updateInstanceConfig(instanceName: string, config: unknown, validation = true): Promise> { // Check existence and get service instance. const inst = this.serviceInstances[instanceName]; if (inst === undefined) { - return error("Service instance doesn't exist."); + return Promise.resolve(error("Service instance doesn't exist.")); } const service = this.services.getService(inst.serviceType); if (service.failed) { - return error("The service of this instance couldn't be found."); + return Promise.resolve(error("The service of this instance couldn't be found.")); } - if (validation || !service.result.requiresNoConfig) { + // If we don't need validation, because we are loading the configuration from disk, we can set it directly + // so that after we return the promise from updateInstanceClient the PersistenceManager can be sure that the + // config has been written. + // Can also be used when there is no configuration needed so that we don't spawn another promise. + if (!validation || service.result.requiresNoConfig) { + inst.config = config; + this.emit("change"); + return this.updateInstanceClient(inst, instanceName, service.result); + } + + // We need to do validation, spawn a Promise + return (async () => { const schemaValid = this.ajv.validate(service.result.schema, config); if (!schemaValid) { return error("Config invalid: " + this.ajv.errorsText()); @@ -170,16 +181,16 @@ export class InstanceManager extends EventEmitter { ); return error("Config invalid: " + err); } - } - // All checks passed. Set config and save it. - inst.config = config; - this.emit("change"); + // All checks passed. Set config and save it. + inst.config = config; + this.emit("change"); - // Update client of this instance using the new config. - const updateResult = await this.updateInstanceClient(inst, instanceName, service.result); + // Update client of this instance using the new config. + const updateResult = await this.updateInstanceClient(inst, instanceName, service.result); - return updateResult; + return updateResult; + })(); } /**