don't throw, just emit error

Author: DTCurrie

src/robot/__tests__/client.spec.ts

@@ -386,12 +386,7 @@ describe('RobotClient', () => {
   });
 
   describe('dial error handling', () => {
-    interface DisconnectedEventCapture {
-      events: unknown[];
-      setupListener: (client: RobotClient) => void;
-    }
-
-    const captureDisconnectedEvents = (): DisconnectedEventCapture => {
+    const captureDisconnectedEvents = () => {
       const events: unknown[] = [];
       const setupListener = (client: RobotClient) => {
         client.on('disconnected', (event) => {
@@ -421,22 +416,38 @@ describe('RobotClient', () => {
       });
     };
 
-    it('should throw an error when both WebRTC and gRPC connections fail', async () => {
+    it('should return client instance when WebRTC connection succeeds', async () => {
+      // Arrange
+      const client = setupClientMocks();
+
+      // Act
+      const result = await client.dial({
+        ...baseDialConfig,
+        noReconnect: true,
+      });
+
+      // Assert
+      expect(result).toBe(client);
+    });
+
+    it('should return client instance even when both WebRTC and gRPC connections fail', async () => {
       // Arrange
       const client = new RobotClient();
       const webrtcError = new Error('WebRTC connection failed');
-      const grpcError = new Error('gRPC connection failed');
+      const { events, setupListener } = captureDisconnectedEvents();
 
       vi.mocked(rpcModule.dialWebRTC).mockRejectedValue(webrtcError);
-      vi.mocked(rpcModule.dialDirect).mockRejectedValue(grpcError);
+      setupListener(client);
 
-      // Act & Assert
-      await expect(
-        client.dial({
-          ...baseDialConfig,
-          noReconnect: true,
-        })
-      ).rejects.toThrow('Failed to connect via all methods');
+      // Act
+      const result = await client.dial({
+        ...baseDialConfig,
+        noReconnect: true,
+      });
+
+      // Assert
+      expect(result).toBe(client);
+      expect(events.length).toBeGreaterThanOrEqual(2);
     });
 
     it('should emit DISCONNECTED event with error when WebRTC fails', async () => {
@@ -449,14 +460,10 @@ describe('RobotClient', () => {
       setupListener(client);
 
       // Act
-      try {
-        await client.dial({
-          ...baseDialConfig,
-          noReconnect: true,
-        });
-      } catch {
-        // Expected to throw
-      }
+      await client.dial({
+        ...baseDialConfig,
+        noReconnect: true,
+      });
 
       // Assert
       expect(events.length).toBeGreaterThanOrEqual(2);
@@ -476,14 +483,10 @@ describe('RobotClient', () => {
       setupListener(client);
 
       // Act
-      try {
-        await client.dial({
-          host: TEST_HOST,
-          noReconnect: true,
-        });
-      } catch {
-        // Expected to throw
-      }
+      await client.dial({
+        host: TEST_HOST,
+        noReconnect: true,
+      });
 
       // Assert
       expect(events.length).toBeGreaterThanOrEqual(1);
@@ -492,83 +495,39 @@ describe('RobotClient', () => {
       expect((errorEvent as { error: Error }).error).toBeInstanceOf(Error);
     });
 
-    it('should emit DISCONNECTED events even for non-Error objects', async () => {
+    it('should emit both errors as DISCONNECTED events when both connections fail', async () => {
       // Arrange
       const client = new RobotClient();
-      const webrtcError = 'string error';
+      const webrtcError = new Error('WebRTC connection failed');
       const { events, setupListener } = captureDisconnectedEvents();
 
       vi.mocked(rpcModule.dialWebRTC).mockRejectedValue(webrtcError);
       setupListener(client);
 
       // Act
-      try {
-        await client.dial({
-          ...baseDialConfig,
-          noReconnect: true,
-        });
-      } catch {
-        // Expected to throw
-      }
+      await client.dial({
+        ...baseDialConfig,
+        noReconnect: true,
+      });
 
       // Assert
       expect(events.length).toBeGreaterThanOrEqual(2);
-      const errorEvent = findEventWithError(events);
-      expect(errorEvent).toBeDefined();
-      expect((errorEvent as { error: Error }).error).toBeInstanceOf(Error);
-    });
-
-    it('should include both errors in the thrown error cause', async () => {
-      // Arrange
-      const client = new RobotClient();
-      const webrtcError = new Error('WebRTC connection failed');
-
-      vi.mocked(rpcModule.dialWebRTC).mockRejectedValue(webrtcError);
-
-      // Act
-      let caughtError: Error | undefined;
-      try {
-        await client.dial({
-          ...baseDialConfig,
-          noReconnect: true,
-        });
-      } catch (error) {
-        caughtError = error as Error;
-      }
-
-      // Assert
-      expect(caughtError).toBeDefined();
-      expect(caughtError).toBeInstanceOf(Error);
-      expect(caughtError!.message).toBe('Failed to connect via all methods');
-      expect(caughtError!.cause).toBeDefined();
-      expect(Array.isArray(caughtError!.cause)).toBe(true);
-      const causes = caughtError!.cause as Error[];
-      expect(causes).toHaveLength(2);
-      expect(causes[0]).toBe(webrtcError);
-      expect(causes[1]).toBeInstanceOf(Error);
+      const webrtcEvent = findEventWithError(
+        events,
+        'WebRTC connection failed'
+      );
+      expect(webrtcEvent).toBeDefined();
+      expect(webrtcEvent).toMatchObject({ error: webrtcError });
     });
 
-    it('should handle non-Error objects thrown from dial methods', async () => {
+    it('should convert non-Error objects to Errors and emit them', async () => {
       // Arrange
       const client = new RobotClient();
       const webrtcError = 'string error';
-      const grpcError = { message: 'object error' };
+      const { events, setupListener } = captureDisconnectedEvents();
 
       vi.mocked(rpcModule.dialWebRTC).mockRejectedValue(webrtcError);
-      vi.mocked(rpcModule.dialDirect).mockRejectedValue(grpcError);
-
-      // Act & Assert
-      await expect(
-        client.dial({
-          ...baseDialConfig,
-          noReconnect: true,
-        })
-      ).rejects.toThrow('Failed to connect via all methods');
-    });
-
-    it('should not throw when WebRTC succeeds', async () => {
-      // Arrange
-      const client = setupClientMocks();
+      setupListener(client);
 
       // Act
       const result = await client.dial({
@@ -578,45 +537,42 @@ describe('RobotClient', () => {
 
       // Assert
       expect(result).toBe(client);
+      expect(events.length).toBeGreaterThanOrEqual(1);
+      const errorEvent = findEventWithError(events);
+      expect(errorEvent).toBeDefined();
+      expect((errorEvent as { error: Error }).error).toBeInstanceOf(Error);
+      expect((errorEvent as { error: Error }).error.message).toBe(
+        'string error'
+      );
     });
 
-    it('should not throw when gRPC succeeds after WebRTC fails', async () => {
+    it('should fallback to gRPC when WebRTC fails and emit WebRTC error', async () => {
       // Arrange
       const client = new RobotClient();
       const webrtcError = new Error('WebRTC connection failed');
+      const { events, setupListener } = captureDisconnectedEvents();
 
+      setupListener(client);
       vi.mocked(rpcModule.dialWebRTC).mockRejectedValue(webrtcError);
       vi.mocked(rpcModule.dialDirect).mockResolvedValue(
         createMockRobotServiceTransport()
       );
 
       // Act
-      // Use a local host so dialDirect validation passes
       const result = await client.dial({
+        ...baseDialConfig,
         host: TEST_LOCAL_HOST,
         noReconnect: true,
       });
 
       // Assert
       expect(result).toBe(client);
-    });
-
-    it('should not throw when only gRPC dial is attempted (no WebRTC config)', async () => {
-      // Arrange
-      const client = new RobotClient();
-      vi.mocked(rpcModule.dialDirect).mockResolvedValue(
-        createMockRobotServiceTransport()
+      expect(events.length).toBeGreaterThanOrEqual(1);
+      const webrtcEvent = findEventWithError(
+        events,
+        'WebRTC connection failed'
       );
-
-      // Act
-      // Use a local host so dialDirect validation passes
-      const result = await client.dial({
-        host: TEST_LOCAL_HOST,
-        noReconnect: true,
-      });
-
-      // Assert
-      expect(result).toBe(client);
+      expect(webrtcEvent).toBeDefined();
     });
   });
 });

src/robot/client.ts

@@ -632,15 +632,12 @@ export class RobotClient extends EventDispatcher implements Robot {
 
     this.currentRetryAttempt = 0;
 
-    let dialWebRTCError: Error | undefined;
-    let dialDirectError: Error | undefined;
-
     // Try to dial via WebRTC first.
     if (isDialWebRTCConf(conf) && !conf.reconnectAbortSignal?.abort) {
       try {
         return await backOff(async () => this.dialWebRTC(conf), backOffOpts);
       } catch (error) {
-        dialWebRTCError =
+        const dialWebRTCError =
           error instanceof Error ? error : new Error(String(error));
         // eslint-disable-next-line no-console
         console.debug('Failed to connect via WebRTC', dialWebRTCError);
@@ -656,7 +653,7 @@ export class RobotClient extends EventDispatcher implements Robot {
       try {
         return await backOff(async () => this.dialDirect(conf), backOffOpts);
       } catch (error) {
-        dialDirectError =
+        const dialDirectError =
           error instanceof Error ? error : new Error(String(error));
         // eslint-disable-next-line no-console
         console.debug('Failed to connect via gRPC', dialDirectError);
@@ -666,12 +663,6 @@ export class RobotClient extends EventDispatcher implements Robot {
       }
     }
 
-    if (dialWebRTCError && dialDirectError) {
-      throw new Error('Failed to connect via all methods', {
-        cause: [dialWebRTCError, dialDirectError],
-      });
-    }
-
     return this;
   }