From 0ec0de982f017b61a145c7a4176718b484834f41 Mon Sep 17 00:00:00 2001
From: Pujit Mehrotra <pujit@lime-technology.com>
Date: Thu, 3 Jul 2025 13:47:48 -0400
Subject: [PATCH] fix(connect): fatal race-condition in websocket disposal
 (#1462)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved error handling during client shutdown to prevent unhandled
exceptions and ensure smoother cleanup.
* Optimized identity state change detection to avoid redundant event
emissions, reducing unnecessary updates when no actual changes occur.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---
- To see the specific tasks where the Asana app for GitHub is being
used, see below:
  - https://app.asana.com/0/0/1210701591479200
---
 .../mothership-proxy/connection.service.ts    | 20 +++++++++++++++----
 .../src/mothership-proxy/graphql.client.ts    | 17 +++++++++++-----
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/packages/unraid-api-plugin-connect/src/mothership-proxy/connection.service.ts b/packages/unraid-api-plugin-connect/src/mothership-proxy/connection.service.ts
index a0f6e2d8e..4736c8097 100644
--- a/packages/unraid-api-plugin-connect/src/mothership-proxy/connection.service.ts
+++ b/packages/unraid-api-plugin-connect/src/mothership-proxy/connection.service.ts
@@ -3,10 +3,11 @@ import { ConfigService } from '@nestjs/config';
 import { EventEmitter2 } from '@nestjs/event-emitter';
 import type { OutgoingHttpHeaders } from 'node:http2';
 
+import { isEqual } from 'lodash-es';
 import { Subscription } from 'rxjs';
-import { bufferTime, filter } from 'rxjs/operators';
+import { debounceTime, filter } from 'rxjs/operators';
 
-import { ConnectionMetadata, MinigraphStatus, MyServersConfig } from '../config/connect.config.js';
+import { ConnectionMetadata, MinigraphStatus } from '../config/connect.config.js';
 import { EVENTS } from '../helper/nest-tokens.js';
 
 interface MothershipWebsocketHeaders extends OutgoingHttpHeaders {
@@ -58,6 +59,7 @@ export class MothershipConnectionService implements OnModuleInit, OnModuleDestro
     };
 
     private identitySubscription: Subscription | null = null;
+    private lastIdentity: Partial<IdentityState> | null = null;
     private metadataChangedSubscription: Subscription | null = null;
 
     constructor(
@@ -83,12 +85,22 @@ export class MothershipConnectionService implements OnModuleInit, OnModuleDestro
         this.identitySubscription = this.configService.changes$
             .pipe(
                 filter((change) => Object.values(this.configKeys).includes(change.path)),
-                bufferTime(25)
+                // debouncing is necessary here (instead of buffering/batching) to prevent excess emissions
+                // because the store.* config values will change frequently upon api boot
+                debounceTime(25)
             )
             .subscribe({
                 next: () => {
+                    const { state } = this.getIdentityState();
+                    if (isEqual(state, this.lastIdentity)) {
+                        this.logger.debug('Identity unchanged; skipping event emission');
+                        return;
+                    }
+                    this.lastIdentity = structuredClone(state);
                     const success = this.eventEmitter.emit(EVENTS.IDENTITY_CHANGED);
-                    if (!success) {
+                    if (success) {
+                        this.logger.debug('Emitted IDENTITY_CHANGED event');
+                    } else {
                         this.logger.warn('Failed to emit IDENTITY_CHANGED event');
                     }
                 },
diff --git a/packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts b/packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts
index 0817765a9..99be2a132 100644
--- a/packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts
+++ b/packages/unraid-api-plugin-connect/src/mothership-proxy/graphql.client.ts
@@ -116,16 +116,23 @@ export class MothershipGraphqlClientService implements OnModuleInit, OnModuleDes
      */
     async clearInstance(): Promise<void> {
         if (this.apolloClient) {
-            await this.apolloClient.clearStore();
-            // some race condition causes apolloClient to be null here upon api shutdown?
-            this.apolloClient?.stop();
+            try {
+                await this.apolloClient.clearStore();
+                // some race condition causes apolloClient to be null here upon api shutdown?
+                this.apolloClient?.stop();
+            } catch (error) {
+                this.logger.warn(error, 'Error clearing apolloClient');
+            }
             this.apolloClient = null;
         }
 
         if (this.wsClient) {
             this.clearClientEventHandlers();
-            this.wsClient.terminate();
-            await this.wsClient.dispose();
+            try {
+                await this.wsClient.dispose();
+            } catch (error) {
+                this.logger.warn(error, 'Error disposing of wsClient');
+            }
             this.wsClient = null;
         }