C2-25226: OpenFin disconnection handler (#2274)

- `openfin.init` allows to provide a disconnection handler - this disconnection handler is stored on SSF side - the openfin handler forward the disconnection event through a new `openfin-disconnection` event - SSF calls the disconnection handler when a new `openfin-disconnection` event is received Minor: - in the openfin handler, a call to reset is now done before trying to connect to OpenFin - removed the timeout log in the timeout promise, the log will be shown if the timeout promise rejects first (or in case of error)
finos · Jan 31, 2025 · 9959873 · 9959873
1 parent 17d1a6a
commit 9959873
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 79 deletions.
diff --git a/spec/openfinHandler.spec.ts b/spec/openfinHandler.spec.ts
@@ -35,10 +35,13 @@ jest.mock('../src/app/config-handler', () => ({
   },
 }));
 
+const mockSend = jest.fn();
 jest.mock('../src/app/window-handler', () => {
   return {
     windowHandler: {
-      getMainWebContents: jest.fn(),
+      getMainWebContents: jest.fn(() => ({
+        send: mockSend,
+      })),
     },
   };
 });
@@ -163,17 +166,23 @@ describe('Openfin', () => {
     expect(fireIntentSpy).toHaveBeenCalledTimes(1);
   });
 
-  it('should register an intent handler', async () => {
+  it('should register an intent handler and trigger intent handler on intent received', async () => {
+    const intentName = 'my-intent';
     const connectSyncMock = await connectMock.Interop.connectSync();
-    const intentHandlerRegistrationSpy = jest.spyOn(
-      connectSyncMock,
-      'registerIntentHandler',
-    );
 
     await openfinHandler.connect();
-    await openfinHandler.registerIntentHandler('my-intent');
+    await openfinHandler.registerIntentHandler(intentName);
+
+    expect(connectSyncMock.registerIntentHandler).toHaveBeenCalledTimes(1);
+
+    const intentHandler =
+      connectSyncMock.registerIntentHandler.mock.calls[0][0];
+    intentHandler('intent-data');
 
-    expect(intentHandlerRegistrationSpy).toHaveBeenCalledTimes(1);
+    expect(mockSend).toHaveBeenCalledWith(
+      'openfin-intent-received',
+      'intent-data',
+    );
   });
 
   it('should join a context group', async () => {
@@ -253,4 +262,25 @@ describe('Openfin', () => {
 
     expect(removeFromContextGroupSpy).toHaveBeenCalledTimes(1);
   });
+
+  it('should trigger disconnection handler when disconnected', async () => {
+    const disconnectionEvent = {
+      type: 'type',
+      topic: 'topic',
+      brokerName: 'broken-name',
+    };
+
+    const connectSyncMock = await connectMock.Interop.connectSync();
+
+    await openfinHandler.connect();
+
+    const disconnectionHandler =
+      connectSyncMock.onDisconnection.mock.calls[0][0];
+    disconnectionHandler(disconnectionEvent);
+
+    expect(mockSend).toHaveBeenCalledWith(
+      'openfin-disconnection',
+      disconnectionEvent,
+    );
+  });
 });
diff --git a/src/app/openfin-handler.ts b/src/app/openfin-handler.ts
@@ -1,6 +1,6 @@
 /// <reference types="@openfin/core/fin" />
 
-import { connect } from '@openfin/node-adapter';
+import { connect, NodeFin } from '@openfin/node-adapter';
 import { randomUUID, UUID } from 'crypto';
 import { logger } from '../common/openfin-logger';
 import { config, IConfig } from './config-handler';
@@ -13,33 +13,35 @@ export class OpenfinHandler {
   private interopClient: OpenFin.InteropClient | undefined;
   private intentHandlerSubscriptions: Map<UUID, any> = new Map();
   private isConnected: boolean = false;
-  private fin: any;
+  private fin: NodeFin | undefined;
 
   /**
    * Connection to interop broker
    */
   public async connect() {
+    this.reset();
+
     const { openfin }: IConfig = config.getConfigFields(['openfin']);
     if (!openfin) {
       logger.error('openfin-handler: missing openfin params to connect.');
       return { isConnected: false };
     }
-    logger.info('openfin-handler: connecting');
+    logger.info('openfin-handler: connecting...');
+
     const parsedTimeoutValue = parseInt(openfin.connectionTimeout, 10);
     const timeoutValue = isNaN(parsedTimeoutValue)
       ? TIMEOUT_THRESHOLD
       : parsedTimeoutValue;
     const connectionTimeoutPromise = new Promise((_, reject) =>
-      setTimeout(() => {
-        logger.error(
-          `openfin-handler: Connection timeout after ${
-            timeoutValue / 1000
-          } seconds`,
-        );
-        return reject(
-          new Error(`Connection timeout after ${timeoutValue / 1000} seconds`),
-        );
-      }, timeoutValue),
+      setTimeout(
+        () =>
+          reject(
+            new Error(
+              `connection timeout after ${timeoutValue / 1000} seconds`,
+            ),
+          ),
+        timeoutValue,
+      ),
     );
 
     const connectionPromise = (async () => {
@@ -49,28 +51,21 @@ export class OpenfinHandler {
             uuid: openfin.uuid,
             licenseKey: openfin.licenseKey,
             runtime: {
-              version: openfin.runtimeVersion,
+              version: openfin.runtimeVersion || 'stable',
             },
           });
         }
 
         logger.info(
-          'openfin-handler: connection established to Openfin runtime',
+          'openfin-handler: connection established to openfin runtime',
         );
         logger.info(
-          `openfin-handler: starting connection to interop broker using channel ${openfin.channelName}`,
+          `openfin-handler: starting connection to interop broker using channel ${openfin.channelName}...`,
         );
 
         this.interopClient = this.fin.Interop.connectSync(openfin.channelName);
         this.isConnected = true;
-
-        this.interopClient?.onDisconnection((event) => {
-          const { brokerName } = event;
-          logger.warn(
-            `openfin-handler: Disconnected from Interop Broker ${brokerName}`,
-          );
-          this.clearSubscriptions();
-        });
+        this.interopClient?.onDisconnection(this.disconnectionHandler);
 
         return true;
       } catch (error) {
@@ -87,7 +82,7 @@ export class OpenfinHandler {
       return { isConnected };
     } catch (error) {
       logger.error(
-        'openfin-handler: error or timeout while connecting: ',
+        'openfin-handler: error or timeout while connecting:',
         error,
       );
       return { isConnected: false };
@@ -105,23 +100,23 @@ export class OpenfinHandler {
    * Adds an intent handler for incoming intents
    */
   public async registerIntentHandler(intentName: string): Promise<UUID> {
-    const unsubscriptionCallback =
-      await this.interopClient?.registerIntentHandler(
-        this.intentHandler,
-        intentName,
-      );
+    const subscription = await this.interopClient?.registerIntentHandler(
+      this.intentHandler,
+      intentName,
+    );
+
     const uuid = randomUUID();
-    this.intentHandlerSubscriptions.set(uuid, unsubscriptionCallback);
+    this.intentHandlerSubscriptions.set(uuid, subscription);
     return uuid;
   }
 
   /**
    * Removes an intent handler for a given intent
    */
   public async unregisterIntentHandler(uuid: UUID) {
-    const unsubscriptionCallback = this.intentHandlerSubscriptions.get(uuid);
+    const subscription = this.intentHandlerSubscriptions.get(uuid);
 
-    const response = await unsubscriptionCallback.unsubscribe();
+    const response = await subscription.unsubscribe();
     this.intentHandlerSubscriptions.delete(uuid);
     return response;
   }
@@ -155,23 +150,21 @@ export class OpenfinHandler {
   }
 
   /**
-   * Clears all openfin subscriptions
+   * Reset connection status, interop client, and existing subscriptions (if any).
    */
-  public clearSubscriptions() {
+  public reset() {
     this.isConnected = false;
     this.interopClient = undefined;
-    this.intentHandlerSubscriptions.forEach(
-      (unsubscriptionCallback, intent) => {
-        try {
-          unsubscriptionCallback.unsubscribe();
-        } catch (e) {
-          logger.error(
-            `openfin-handler: Error unsubscribing from intent ${intent}:`,
-            e,
-          );
-        }
-      },
-    );
+    this.intentHandlerSubscriptions.forEach((subscriptions, intent) => {
+      try {
+        subscriptions.unsubscribe();
+      } catch (e) {
+        logger.error(
+          `openfin-handler: error unsubscribing from intent ${intent}:`,
+          e,
+        );
+      }
+    });
     this.intentHandlerSubscriptions.clear();
   }
 
@@ -227,10 +220,25 @@ export class OpenfinHandler {
     };
   }
 
+  /**
+   * Forward intent to main window when intent is received
+   */
   private intentHandler = (intent: any) => {
     logger.info('openfin-handler: intent received - ', intent);
-    const mainWebContents = windowHandler.getMainWebContents();
-    mainWebContents?.send('intent-received', intent);
+    windowHandler.getMainWebContents()?.send('openfin-intent-received', intent);
+  };
+
+  /**
+   * Forward disconnection event to main window when disconnected from Interop Broker
+   */
+  private disconnectionHandler = (
+    event: OpenFin.InteropBrokerDisconnectionEvent,
+  ) => {
+    logger.warn(
+      `openfin-handler: disconnected from interop broker ${event.brokerName}`,
+    );
+    windowHandler.getMainWebContents()?.send('openfin-disconnection', event);
+    this.reset();
   };
 }
 

diff --git a/src/app/window-utils.ts b/src/app/window-utils.ts
@@ -476,7 +476,7 @@ export const sanitize = (windowName: string): void => {
     // reset the badge count whenever an user refreshes the electron client
     showBadgeCount(0);
     // Clear all openfin subscriptions
-    openfinHandler.clearSubscriptions();
+    openfinHandler.reset();
     // Terminates the screen snippet process and screen share indicator frame on reload
     if (!isMac || !isLinux) {
       logger.info(

diff --git a/src/renderer/ssf-api.ts b/src/renderer/ssf-api.ts
@@ -67,14 +67,15 @@ export interface ILocalObject {
   c9MessageCallback?: (status: IShellStatus) => void;
   updateMyPresenceCallback?: (presence: EPresenceStatusCategory) => void;
   phoneNumberCallback?: (arg: string) => void;
-  intentsCallbacks: Map<string, Map<UUID, any>>;
+  openfinIntentCallbacks: Map<string, Map<UUID, any>>; // by intent name, then by callback id
+  openfinDisconnectionCallback?: (event?: any) => void;
   writeImageToClipboard?: (blob: string) => void;
   getHelpInfo?: () => Promise<IPodSettingsClientSpecificSupportLink>;
 }
 
 const local: ILocalObject = {
   ipcRenderer,
-  intentsCallbacks: new Map(),
+  openfinIntentCallbacks: new Map(),
 };
 
 const notificationActionCallbacks = new Map<
@@ -958,13 +959,17 @@ export class SSFApi {
   /**
    * Openfin Interop client initialization
    */
-  public async openfinInit() {
+  public async openfinInit(options?: {
+    onDisconnection?: (event: any) => void;
+  }): Promise<void> {
     const connectionStatus = await local.ipcRenderer.invoke(
       apiName.symphonyApi,
-      {
-        cmd: apiCmds.openfinConnect,
-      },
+      { cmd: apiCmds.openfinConnect },
     );
+
+    local.openfinIntentCallbacks.clear();
+    local.openfinDisconnectionCallback = options?.onDisconnection;
+
     return connectionStatus;
   }
 
@@ -1046,12 +1051,12 @@ export class SSFApi {
       cmd: apiCmds.openfinRegisterIntentHandler,
       intentName,
     });
-    if (local.intentsCallbacks.has(intentName)) {
-      local.intentsCallbacks.get(intentName)?.set(uuid, intentHandler);
+    if (local.openfinIntentCallbacks.has(intentName)) {
+      local.openfinIntentCallbacks.get(intentName)?.set(uuid, intentHandler);
     } else {
       const innerMap = new Map();
       innerMap.set(uuid, intentHandler);
-      local.intentsCallbacks.set(intentName, innerMap);
+      local.openfinIntentCallbacks.set(intentName, innerMap);
     }
     return uuid;
   }
@@ -1061,7 +1066,7 @@ export class SSFApi {
    * @param UUID
    */
   public async openfinUnregisterIntentHandler(callbackId: UUID): Promise<void> {
-    for (const innerMap of local.intentsCallbacks.values()) {
+    for (const innerMap of local.openfinIntentCallbacks.values()) {
       if (innerMap.has(callbackId)) {
         innerMap.delete(callbackId);
         break;
@@ -1460,16 +1465,23 @@ local.ipcRenderer.on(
   },
 );
 
-local.ipcRenderer.on('intent-received', (_event: Event, intent: any) => {
-  if (
-    typeof intent.name === 'string' &&
-    local.intentsCallbacks.has(intent.name)
-  ) {
-    const uuidCallbacks = local.intentsCallbacks.get(intent.name);
-    uuidCallbacks?.forEach((callbacks, _uuid) => {
-      callbacks(intent.context);
-    });
-  }
+local.ipcRenderer.on(
+  'openfin-intent-received',
+  (_event: Event, intent: any) => {
+    if (
+      typeof intent.name === 'string' &&
+      local.openfinIntentCallbacks.has(intent.name)
+    ) {
+      const uuidCallbacks = local.openfinIntentCallbacks.get(intent.name);
+      uuidCallbacks?.forEach((callbacks, _uuid) => {
+        callbacks(intent.context);
+      });
+    }
+  },
+);
+
+local.ipcRenderer.on('openfin-disconnection', (_event: Event) => {
+  local.openfinDisconnectionCallback?.();
 });
 
 // Invoked whenever the app is reloaded/navigated
@@ -1480,7 +1492,7 @@ const sanitize = (): void => {
       windowName: window.name,
     });
   }
-  local.intentsCallbacks = new Map();
+  local.openfinIntentCallbacks = new Map();
 };
 
 // listens for the online/offline events and updates the main process