Unregister handlers of bundle when assigning an other instance

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();
+    }
 }

nodecg-io-slack/extension/index.ts

@@ -41,7 +41,12 @@ class SlackService extends ServiceBundle<SlackServiceConfig, SlackServiceClient>
         }
     }
 
-    stopClient(_client: SlackServiceClient): void {
-        // Not supported by the client
+    stopClient(client: SlackServiceClient): void {
+        // Not supported by the client, at least remove all listeners
+        this.removeHandlers(client);
+    }
+
+    removeHandlers(client: SlackServiceClient): void {
+        client.getNativeClient().removeAllListeners();
     }
 }

nodecg-io-spotify/extension/index.ts

@@ -108,6 +108,6 @@ class SpotifyService extends ServiceBundle<SpotifyServiceConfig, SpotifyServiceC
     }
 
     stopClient(_client: SpotifyServiceClient): void {
-        // Not supported from the client
+        // Not needed, it is just a stateless http client
     }
 }

nodecg-io-streamdeck/extension/index.ts

@@ -53,4 +53,7 @@ class StreamdeckServiceBundle extends ServiceBundle<StreamdeckServiceConfig, Str
     stopClient(client: StreamdeckServiceClient): void {
         client.getNativeClient().close();
     }
+
+    // Can't remove handlers for up/down/error, so re-create the client to get rid of the listeners
+    reCreateClientToRemoveHandlers = true;
 }

nodecg-io-streamelements/extension/index.ts

@@ -31,4 +31,8 @@ class StreamElementsService extends ServiceBundle<StreamElementsServiceConfig, S
     stopClient(client: StreamElementsServiceClient) {
         client.getNativeClient().close();
     }
+
+    removeHandlers(client: StreamElementsServiceClient) {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-telegram/extension/index.ts

@@ -55,4 +55,10 @@ class TelegramService extends ServiceBundle<TelegramServiceConfig, TelegramServi
             client.getNativeClient().closeWebHook();
         }
     }
+
+    removeHandlers(client: TelegramServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+        client.getNativeClient().clearTextListeners();
+        client.getNativeClient().clearReplyListeners();
+    }
 }

nodecg-io-telegram/package.json

@@ -35,8 +35,8 @@
         "typescript": "^4.1.3"
     },
     "dependencies": {
-        "@types/node-telegram-bot-api": "^0.50.4",
-        "node-telegram-bot-api": "^0.50.0",
+        "@types/node-telegram-bot-api": "^0.51.0",
+        "node-telegram-bot-api": "^0.51.0",
         "nodecg-io-core": "^0.1.0"
     }
 }

nodecg-io-tiane/extension/index.ts

@@ -36,4 +36,8 @@ class TianeService extends ServiceBundle<TianeServiceConfig, TianeServiceClient>
         client.getNativeClient().close();
         this.nodecg.log.info("Disconnected from TIANE.");
     }
+
+    removeHandlers(client: TianeServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-twitch-chat/extension/index.ts

@@ -28,10 +28,13 @@ class TwitchService extends ServiceBundle<TwitchServiceConfig, TwitchServiceClie
     }
 
     stopClient(client: TwitchServiceClient): void {
-        client.getNativeClient().removeListener();
         client
             .getNativeClient()
             .quit()
             .then(() => this.nodecg.log.info("Stopped twitch client successfully."));
     }
+
+    removeHandlers(client: TwitchServiceClient): void {
+        client.getNativeClient().removeListener();
+    }
 }

nodecg-io-twitch-pubsub/extension/index.ts

@@ -28,4 +28,9 @@ class TwitchPubSubService extends ServiceBundle<PubSubServiceConfig, PubSubServi
     stopClient(_: PubSubServiceClient): void {
         // Not possible
     }
+
+    // Pubsub has no methods to close the connection or remove the handler.
+    // It has no way to access the underlying client too, so handlers that are setup once will currently live forever.
+    // TODO: implement a way to at least stop the client, removing handlers would be nice too
+    recreateClientToRemoveHandlers = true;
 }

nodecg-io-websocket-client/extension/index.ts

@@ -65,4 +65,8 @@ class WSClientService extends ServiceBundle<WSClientServiceConfig, WSClientServi
     stopClient(client: WSClientServiceClient): void {
         client.getNativeClient().close();
     }
+
+    removeHandlers(client: WSClientServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+    }
 }

nodecg-io-websocket-server/extension/index.ts

@@ -58,4 +58,13 @@ class WSServerService extends ServiceBundle<WSServerServiceConfig, WSServerServi
     stopClient(client: WSServerServiceClient): void {
         client.getNativeClient().close();
     }
+
+    removeHandlers(client: WSServerServiceClient): void {
+        client.getNativeClient().removeAllListeners();
+        // Drop all clients so that they have to reconnect and the bundles using
+        // ws.on("connection", ...) handlers are re-run
+        client.getNativeClient().clients.forEach((client) => {
+            client.close();
+        });
+    }
 }