Merge pull request #122 from codeoverflow-org/feature/9-remove-client-listeners

Unregister handlers of bundle when assigning an other instance

Author: hlxid

nodecg-io-core/extension/bundleManager.ts

@@ -81,14 +81,17 @@ export class BundleManager extends EventEmitter {
         if (svcDependency === undefined) {
             return error(`Bundle "${bundleName} doesn't depend on the "${instance.serviceType}" service.`);
         }
+        const oldInstance = svcDependency.serviceInstance;
 
         // Update service instance of service dependency, remove client update callback from old service instance (if applicable)
         // and add the callback to the new instance.
         svcDependency.serviceInstance = instanceName;
 
         // Let the bundle update his reference to the client
         svcDependency.clientUpdateCallback(instance.client);
+
         this.emit("change");
+        this.emit("reregisterInstance", oldInstance);
         return emptySuccess();
     }
 
@@ -105,10 +108,15 @@ export class BundleManager extends EventEmitter {
         const svcDependency = bundle?.find((svcDep) => svcDep.serviceType === serviceType);
 
         if (svcDependency !== undefined) {
+            const oldInstance = svcDependency.serviceInstance;
+
             // Unset service instance and let the bundle know that it hasn't access to this service anymore.
             svcDependency.serviceInstance = undefined;
             svcDependency.clientUpdateCallback(undefined);
+
             this.emit("change");
+            this.emit("reregisterInstance", oldInstance);
+
             return true;
         }
 

nodecg-io-core/extension/instanceManager.ts

@@ -19,6 +19,9 @@ export class InstanceManager extends EventEmitter {
         private readonly bundles: BundleManager,
     ) {
         super();
+        bundles.on("reregisterInstance", (serviceInstance?: string) =>
+            this.reregisterHandlersOfInstance(serviceInstance),
+        );
     }
 
     /**
@@ -199,17 +202,14 @@ export class InstanceManager extends EventEmitter {
 
                 // Check if a error happened while creating the client
                 if (client.failed) {
-                    this.nodecg.log.error(
-                        `The "${inst.serviceType}" service produced an error while creating a client: ${client.errorMessage}`,
-                    );
-                    inst.client = undefined;
+                    throw client.errorMessage; // Error logging happens in catch block
                 } else {
                     // Update service instance object
                     inst.client = client.result;
                 }
             } catch (err) {
                 this.nodecg.log.error(
-                    `The "${inst.serviceType}" service function produced an error while creating a client: ${err}`,
+                    `The "${inst.serviceType}" service produced an error while creating a client: ${err}`,
                 );
                 inst.client = undefined;
             }
@@ -228,4 +228,46 @@ export class InstanceManager extends EventEmitter {
             }
         }
     }
+
+    /**
+     * Removes all handlers from the service client of the instance and lets bundles readd their handlers.
+     * @param instanceName the name of the instance which handlers should be re-registred
+     */
+    private reregisterHandlersOfInstance(instanceName?: string): void {
+        if (!instanceName) return;
+
+        const inst = this.getServiceInstance(instanceName);
+        if (!inst) {
+            this.nodecg.log.error(`Can't re-register handlers of instance "${instanceName}": instance not found`);
+            return;
+        }
+
+        const svc = this.services.getService(inst.serviceType);
+        if (svc.failed) {
+            this.nodecg.log.error(
+                `Can't reregister handlers of instance "${instanceName}": can't get service: ${svc.errorMessage}`,
+            );
+            return;
+        }
+
+        // Client should be recreated because the Service has no way to reset the handlers.
+        if (svc.result.reCreateClientToRemoveHandlers) {
+            this.updateInstanceClient(inst, instanceName, svc.result);
+            return;
+        }
+
+        if (!svc.result.removeHandlers) return; // Service provides no way to remove handlers, thus this service has no handlers
+
+        // Remove handlers
+        try {
+            svc.result.removeHandlers(inst.client);
+        } catch (err) {
+            this.nodecg.log.error(
+                `Can't re-register handlers of instance "${instanceName}": error while removing handlers: ${err.toString()}`,
+            );
+        }
+        // Readd handlers by running the `onAvailable` function of all bundles
+        // that are using this service instance.
+        this.bundles.handleInstanceUpdate(inst, instanceName);
+    }
 }

nodecg-io-core/extension/serviceBundle.ts

@@ -78,6 +78,28 @@ export abstract class ServiceBundle<R, C extends ServiceClient<unknown>> impleme
      */
     abstract stopClient(client: C): void;
 
+    /**
+     * Removes all handlers from a service client.
+     * This is used when a bundle no longer uses a service client it still has its handlers registered.
+     * Then this function is called that should remove all handlers
+     * and then all bundles that are still using this client will asked to re-register their handlers
+     * by running the onAvailable callback of the specific bundle.
+     *
+     * Can be left unimplemented if the serivce doesn't has any handlers e.g. a http wrapper
+     * @param client the client of which all handlers should be removed
+     */
+    removeHandlers?(client: C): void;
+
+    /**
+     * This flag can be enabled by services if they can't implement removeHandlers but also have some handlers that
+     * should be reset if a bundleDependency has been changed.
+     * It gets rid of the handlers by stopping the client and creating a new one, to which then only the
+     * now wanted handlers get registered (e.g. if a bundle doesn't uses this service anymore but another still does).
+     * Not ideal, but if your service can't implement removeHandlers for some reason it is still better than
+     * having dangling handlers that still fire eventho they shouldn't.
+     */
+    reCreateClientToRemoveHandlers = false;
+
     private readSchema(pathSegments: string[]): unknown {
         const joinedPath = path.resolve(...pathSegments);
         try {

nodecg-io-core/extension/types.d.ts

@@ -60,6 +60,28 @@ export interface Service<R, C extends ServiceClient<unknown>> {
      * @param client the client that needs to be stopped.
      */
     readonly stopClient(client: C): void;
+
+    /**
+     * Removes all handlers from a service client.
+     * This is used when a bundle no longer uses a service client it still has its handlers registered.
+     * Then this function is called that should remove all handlers
+     * and then all bundles that are still using this client will asked to re-register their handlers
+     * by running the onAvailable callback of the specific bundle.
+     *
+     * Can be left unimplemented if the serivce doesn't has any handlers e.g. a http wrapper
+     * @param client the client of which all handlers should be removed
+     */
+    readonly removeHandlers?(client: C): void;
+
+    /**
+     * This flag can be enabled by services if they can't implement removeHandlers but also have some handlers that
+     * should be reset if a bundleDependency has been changed.
+     * It gets rid of the handlers by stopping the client and creating a new one, to which then only the
+     * now wanted handlers get registered (e.g. if a bundle doesn't uses this service anymore but another still does).
+     * Not ideal, but if your service can't implement removeHandlers for some reason it is still better than
+     * having dangling handlers that still fire eventho they shouldn't.
+     */
+    reCreateClientToRemoveHandlers: boolean;
 }
 
 /**

nodecg-io-discord/extension/index.ts

@@ -38,4 +38,8 @@ class DiscordService extends ServiceBundle<DiscordServiceConfig, DiscordServiceC
         const rawClient = client.getNativeClient();
         rawClient.destroy();
     }
+
+    removeHandlers(client: DiscordServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-irc/extension/index.ts

@@ -62,6 +62,10 @@ class IRCService extends ServiceBundle<IRCServiceConfig, IRCServiceClient> {
             this.nodecg.log.info("Stopped IRC client successfully.");
         });
     }
+
+    removeHandlers(client: IRCServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }
 
 function sendMessage(client: IRCClient, target: string, message: string): void {

nodecg-io-midi-input/extension/index.ts

@@ -59,4 +59,8 @@ class MidiService extends ServiceBundle<MidiInputServiceConfig, MidiInputService
     stopClient(client: MidiInputServiceClient): void {
         client.getNativeClient().close();
     }
+
+    removeHandlers(client: MidiInputServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-obs/extension/index.ts

@@ -47,4 +47,8 @@ class OBSService extends ServiceBundle<OBSServiceConfig, OBSServiceClient> {
     stopClient(client: OBSServiceClient) {
         client.getNativeClient().disconnect();
     }
+
+    removeHandlers(client: OBSServiceClient) {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-sacn-receiver/extension/index.ts

@@ -31,4 +31,8 @@ class SacnReceiverService extends ServiceBundle<SacnReceiverServiceConfig, SacnR
         client.getNativeClient().close();
         this.nodecg.log.info("Stopped sACN Receiver successfully.");
     }
+
+    removeHandlers(client: SacnReceiverServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-serial/extension/index.ts

@@ -24,4 +24,8 @@ class SerialService extends ServiceBundle<SerialServiceConfig, SerialServiceClie
     stopClient(client: SerialServiceClient): void {
         client.close();
     }
+
+    removeHandlers(client: SerialServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }