fix(cdp): enhance error handling on the target attaching failure (#179)

closes #173

src/Plugin.ts

@@ -10,6 +10,11 @@ import type {
 import { HarBuilder, NetworkIdleMonitor, NetworkRequest } from './network';
 import { ErrorUtils } from './utils/ErrorUtils';
 import type { Connection, ConnectionFactory } from './cdp';
+import {
+  ADDRESS_OPTION_NAME,
+  PORT_OPTION_NAME,
+  SUPPORTED_BROWSERS
+} from './constants';
 import { join } from 'path';
 import { EOL } from 'os';
 import { promisify } from 'util';
@@ -35,8 +40,6 @@ export class Plugin {
   private networkObservable?: Observer<NetworkRequest>;
   private addr?: Addr;
   private _connection?: Connection;
-  private readonly PORT_OPTION_NAME = '--remote-debugging-port';
-  private readonly ADDRESS_OPTION_NAME = '--remote-debugging-address';
 
   constructor(
     private readonly logger: Logger,
@@ -137,18 +140,16 @@ export class Plugin {
   }
 
   private parseElectronSwitches(browser: Cypress.Browser): string[] {
-    if (
-      !process.env.ELECTRON_EXTRA_LAUNCH_ARGS?.includes(this.PORT_OPTION_NAME)
-    ) {
+    if (!process.env.ELECTRON_EXTRA_LAUNCH_ARGS?.includes(PORT_OPTION_NAME)) {
       this.logger
-        .err(`The '${browser.name}' browser was detected, however, the required '${this.PORT_OPTION_NAME}' command line switch was not provided. 
+        .err(`The '${browser.name}' browser was detected, however, the required '${PORT_OPTION_NAME}' command line switch was not provided. 
 This switch is necessary to enable remote debugging over HTTP on the specified port. 
 
 Please refer to the documentation:
   - https://www.electronjs.org/docs/latest/api/command-line-switches#--remote-debugging-portport
   - https://docs.cypress.io/api/plugins/browser-launch-api#Modify-Electron-app-switches`);
       throw new Error(
-        `Missing '${this.PORT_OPTION_NAME}' command line switch for Electron browser`
+        `Missing '${PORT_OPTION_NAME}' command line switch for Electron browser`
       );
     }
 
@@ -209,7 +210,7 @@ Please refer to the documentation:
   }
 
   private isSupportedBrowser(browser: Cypress.Browser): boolean {
-    return ['chromium'].includes(browser?.family);
+    return SUPPORTED_BROWSERS.includes(browser?.family);
   }
 
   private ensureRdpAddrArgs(args: string[]): string[] {
@@ -222,19 +223,19 @@ Please refer to the documentation:
 
     return [
       ...args,
-      `${this.PORT_OPTION_NAME}=${port}`,
-      `${this.ADDRESS_OPTION_NAME}=${host}`
+      `${PORT_OPTION_NAME}=${port}`,
+      `${ADDRESS_OPTION_NAME}=${host}`
     ];
   }
 
   private extractAddrFromArgs(args: string[]): Partial<Addr> {
     const port: string | undefined = this.findAndParseIfPossible(
       args,
-      this.PORT_OPTION_NAME
+      PORT_OPTION_NAME
     );
     const host: string | undefined = this.findAndParseIfPossible(
       args,
-      this.ADDRESS_OPTION_NAME
+      ADDRESS_OPTION_NAME
     );
 
     let addr: { port?: number; host?: string } = {};

src/cdp/CDPConnection.spec.ts

@@ -3,7 +3,7 @@ import { Logger } from '../utils/Logger';
 import { RetryStrategy } from './RetryStrategy';
 import {
   CONNECTED,
-  CONNECTION_IS_NOT_DEFINED,
+  CONNECTION_NOT_ESTABLISHED,
   DISCONNECTED,
   FAILED_ATTEMPT_TO_CONNECT
 } from './messages';
@@ -180,7 +180,7 @@ describe('CDPConnection', () => {
       // act
       const act = () => sut.discoverNetwork();
       // assert
-      expect(act).toThrow(CONNECTION_IS_NOT_DEFINED);
+      expect(act).toThrow(CONNECTION_NOT_ESTABLISHED);
     });
 
     it('should create a new network monitor', async () => {

src/cdp/CDPConnection.ts

@@ -3,7 +3,7 @@ import { Logger } from '../utils/Logger';
 import {
   ATTEMPT_TO_CONNECT,
   CONNECTED,
-  CONNECTION_IS_NOT_DEFINED,
+  CONNECTION_NOT_ESTABLISHED,
   DISCONNECTED,
   FAILED_ATTEMPT_TO_CONNECT,
   FAILED_TO_CONNECT
@@ -67,11 +67,11 @@ export class CDPConnection implements Connection {
 
   public discoverNetwork(): Network {
     if (!this.cdp) {
-      throw new Error(CONNECTION_IS_NOT_DEFINED);
+      throw new Error(CONNECTION_NOT_ESTABLISHED);
     }
 
     if (!this._network) {
-      this._network = new DefaultNetwork(this.cdp);
+      this._network = new DefaultNetwork(this.cdp, this.logger);
     }
 
     return this._network;

src/cdp/DefaultNetwork.spec.ts

@@ -1,38 +1,44 @@
 import { DefaultNetwork } from './DefaultNetwork';
+import type { Logger } from '../utils/Logger';
+import {
+  TARGET_OR_BROWSER_CLOSED,
+  UNABLE_TO_ATTACH_TO_TARGET
+} from './messages';
 import {
-  mock,
-  instance,
-  when,
-  verify,
   anyFunction,
-  reset,
+  anything,
   deepEqual,
-  anything
+  instance,
+  mock,
+  reset,
+  verify,
+  when
 } from 'ts-mockito';
 import type { Client } from 'chrome-remote-interface';
 import {
+  afterEach,
   beforeEach,
   describe,
-  jest,
-  it,
   expect,
-  afterEach
+  it,
+  jest
 } from '@jest/globals';
 import type Protocol from 'devtools-protocol';
 
 describe('DefaultNetwork', () => {
   const clientMock = mock<Client>();
+  const loggerMock = mock<Logger>();
   const listener = jest.fn();
 
   let sut!: DefaultNetwork;
 
   beforeEach(() => {
-    sut = new DefaultNetwork(instance(clientMock));
+    sut = new DefaultNetwork(instance(clientMock), instance(loggerMock));
   });
 
   afterEach(() => {
     listener.mockReset();
-    reset(clientMock);
+    reset<Client | Logger>(clientMock, loggerMock);
   });
 
   describe('attachToTargets', () => {
@@ -90,6 +96,17 @@ describe('DefaultNetwork', () => {
       ).once();
     });
 
+    it('should ignore a error while subscribing to the security domain', async () => {
+      // arrange
+      when(clientMock.send('Security.enable')).thenReject(
+        new Error('Something went wrong.')
+      );
+      // act
+      const act = sut.attachToTargets(listener);
+      // assert
+      await expect(act).resolves.not.toThrow();
+    });
+
     it('should start tracking sessions', async () => {
       // act
       await sut.attachToTargets(listener);
@@ -274,6 +291,64 @@ describe('DefaultNetwork', () => {
       ).once();
     });
 
+    it('should throw an error when an unexpected error is happened', async () => {
+      // arrange
+      const sessionId = '1';
+      const targetInfo: Protocol.Target.TargetInfo = {
+        targetId: '1',
+        type: 'page',
+        url: '',
+        title: '',
+        attached: true,
+        canAccessOpener: false
+      };
+      let act: ((...args: unknown[]) => Promise<void>) | undefined;
+      when(clientMock.on('Target.attachedToTarget', anyFunction())).thenCall(
+        (_, callback) => (act = callback)
+      );
+      when(
+        clientMock.send(
+          'Runtime.runIfWaitingForDebugger',
+          anything(),
+          sessionId
+        )
+      ).thenReject(new Error('Something went wrong'));
+      await sut.attachToTargets(listener);
+      // act
+      const result = act?.({ sessionId, targetInfo, waitingForDebugger: true });
+      // assert
+      await expect(result).rejects.toThrow();
+      verify(loggerMock.err(UNABLE_TO_ATTACH_TO_TARGET)).once();
+    });
+
+    it.each([{ input: 'Target closed' }, { input: 'Session closed' }])(
+      'should silently handle an error when the $input has been closed',
+      async ({ input }) => {
+        // arrange
+        const sessionId = '1';
+        const targetInfo: Protocol.Target.TargetInfo = {
+          targetId: '1',
+          type: 'page',
+          url: '',
+          title: '',
+          attached: true,
+          canAccessOpener: false
+        };
+        let act: ((...args: unknown[]) => Promise<void>) | undefined;
+        when(clientMock.on('Target.attachedToTarget', anyFunction())).thenCall(
+          (_, callback) => (act = callback)
+        );
+        when(
+          clientMock.send('Network.enable', deepEqual({}), sessionId)
+        ).thenReject(new Error(input));
+        await sut.attachToTargets(listener);
+        // act
+        await act?.({ sessionId, targetInfo, waitingForDebugger: true });
+        // assert
+        verify(loggerMock.debug(TARGET_OR_BROWSER_CLOSED)).once();
+      }
+    );
+
     it('should ignore certificate errors', async () => {
       // arrange
       const eventId = 1;
@@ -325,6 +400,17 @@ describe('DefaultNetwork', () => {
       verify(clientMock.off('Network.webSocketCreated', anyFunction())).never();
     });
 
+    it('should ignore any errors while unsubscribing from the security domain', async () => {
+      // arrange
+      when(clientMock.send('Security.disable')).thenReject(
+        new Error('Something went wrong.')
+      );
+      // act
+      const act = sut.detachFromTargets();
+      // assert
+      await expect(act).resolves.not.toThrow();
+    });
+
     it('should unsubscribe from the security domain', async () => {
       // act
       await sut.detachFromTargets();