fix(devtools-proxy-support): reconnect SSH tunnel after unrecoverable client error COMPASS-8355

packages/devtools-proxy-support/src/ssh.spec.ts

@@ -1,3 +1,4 @@
+import { once } from 'events';
 import { HTTPServerProxyTestSetup } from '../test/helpers';
 import { SSHAgent } from './ssh';
 import { createFetch } from './fetch';
@@ -125,4 +126,71 @@ describe('SSHAgent', function () {
     expect(setup.authHandler).to.have.been.calledTwice;
     expect(setup.canTunnel).to.have.been.calledTwice;
   });
+
+  it('does not crash on unexpected sshClient error events', async function () {
+    agent = new SSHAgent({
+      proxy: `ssh://foo:bar@127.0.0.1:${setup.sshProxyPort}/`,
+    });
+    await agent.initialize();
+    expect(() => {
+      (agent as any).sshClient.emit(
+        'error',
+        new Error('some unexpected ssh error'),
+      );
+    }).not.to.throw();
+    expect((agent as any).connected).to.be.false;
+  });
+
+  it('reconnects with a fresh SSH client after "Instance unusable after fatal error"', async function () {
+    setup.authHandler = sinon.stub().returns(true);
+    agent = new SSHAgent({
+      proxy: `ssh://foo:bar@127.0.0.1:${setup.sshProxyPort}/`,
+    });
+    await agent.initialize();
+    const fetch = createFetch(agent);
+    await fetch('http://example.com/hello');
+
+    // Simulate the ssh2 client entering an unrecoverable "unusable" state
+    // (e.g. the TCP connection was killed mid-stream during hibernate). When
+    // connect() is called on such a client, ssh2 emits 'error' instead of
+    // 'ready', which our new createSshClient() path must handle by discarding
+    // the broken instance and creating a fresh one.
+    const brokenClient = (agent as any).sshClient;
+    brokenClient.connect = function () {
+      process.nextTick(() =>
+        brokenClient.emit(
+          'error',
+          new Error('Instance unusable after fatal error'),
+        ),
+      );
+    };
+    (agent as any).connected = false;
+
+    await fetch('http://example.com/hello');
+    // A fresh client was created and a new SSH handshake performed
+    expect(setup.authHandler).to.have.been.calledTwice;
+  });
+
+  it('reconnects after the underlying connection is destroyed server-side', async function () {
+    setup.authHandler = sinon.stub().returns(true);
+    agent = new SSHAgent({
+      proxy: `ssh://foo:bar@127.0.0.1:${setup.sshProxyPort}/`,
+    });
+    await agent.initialize();
+    const fetch = createFetch(agent);
+    await fetch('http://example.com/hello');
+    expect(setup.authHandler).to.have.been.calledOnce;
+
+    // Simulate the OS killing network connections during hibernate by
+    // forcibly destroying the SSH server's TCP sockets (with a TCP reset)
+    // from the server side, then wait for the agent to notice the loss.
+    const clientClosed = once(agent.logger, 'ssh:client-closed');
+    setup.destroySSHConnections();
+    await clientClosed;
+
+    // The next request must transparently re-establish the SSH connection.
+    const response = await fetch('http://example.com/hello');
+    expect(await response.text()).to.equal('OK /hello');
+    expect(setup.authHandler).to.have.been.calledTwice;
+  });
 });

packages/devtools-proxy-support/src/ssh.ts

@@ -37,7 +37,7 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
   private connected = false;
   private connectingPromise?: Promise<void>;
   private closed = false;
-  private forwardOut: (
+  private forwardOut!: (
     srcIP: string,
     srcPort: number,
     dstIP: string,
@@ -52,21 +52,33 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
     this.logger = logger ?? new EventEmitter().setMaxListeners(Infinity);
     this.proxyOptions = options;
     this.url = new URL(options.proxy ?? '');
-    this.sshClient = new (ssh2().Client)();
-    this.sshClient.on('close', () => {
+    this.sshClient = this.createSshClient();
+  }
+
+  private createSshClient(): SshClient {
+    const client = new (ssh2().Client)();
+    client.on('close', () => {
       this.logger.emit('ssh:client-closed');
       this.connected = false;
     });
-
-    this.forwardOut = promisify(this.sshClient.forwardOut.bind(this.sshClient));
+    client.on('error', () => {
+      // Errors during connection setup are handled through initialize()'s
+      // connectingPromise race, and post-connection errors through _connect()'s
+      // catch block. This listener prevents unhandled 'error' events from
+      // crashing the process when the SSH session dies unexpectedly (e.g. after
+      // the host machine resumes from hibernate).
+      this.connected = false;
+    });
+    this.forwardOut = promisify(client.forwardOut.bind(client));
+    return client;
   }
 
-  async initialize(): Promise<void> {
-    if (this.connected) {
+  async initialize(reinitializeClient = false): Promise<void> {
+    if (this.connected && !reinitializeClient) {
       return;
     }
 
-    if (this.connectingPromise) {
+    if (this.connectingPromise && !reinitializeClient) {
       return this.connectingPromise;
     }
 
@@ -75,6 +87,19 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
       throw new Error('Disconnected.');
     }
 
+    if (reinitializeClient) {
+      // The previous ssh2 Client instance is in an unrecoverable state (e.g.
+      // "Instance unusable after fatal error" after the TCP connection was killed
+      // mid-stream during hibernate). End it to release the keepalive timer
+      // and any other internal handles, then create a fresh instance.
+      // ssh2's end() is a no-op when the underlying socket is already dead
+      // (it guards with isWritable()), so this is safe in all cases.
+      this.connectingPromise = undefined;
+      this.connected = false;
+      this.sshClient.end();
+      this.sshClient = this.createSshClient();
+    }
+
     const sshConnectConfig: ConnectConfig = {
       readyTimeout: 20000,
       keepaliveInterval: 20000,
@@ -117,11 +142,11 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
       this.logger.emit('ssh:failed-connection', {
         error: (err as any)?.stack ?? String(err),
       });
-      delete this.connectingPromise;
+      this.connectingPromise = undefined;
       throw err;
     }
 
-    delete this.connectingPromise;
+    this.connectingPromise = undefined;
     this.connected = true;
     this.logger.emit('ssh:established-connection');
   }
@@ -139,12 +164,14 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
     retriesLeft = 1,
   ): Promise<Duplex> {
     let host = '';
+    let initializedSuccessfully = false;
     try {
       // Using the `host` header matches what proxy-agent does
       host = connectOpts.host || (req.getHeader('host') as string);
       const url = new URL(req.path, `tcp://${host}:${connectOpts.port}`);
 
       await this.initialize();
+      initializedSuccessfully = true;
 
       let sock: Duplex & Partial<Pick<Socket, 'setTimeout'>> =
         await this.forwardOut('127.0.0.1', 0, url.hostname, +url.port);
@@ -161,9 +188,17 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
       }
       return sock;
     } catch (err: unknown) {
-      const retryableError = /Not connected|Channel open failure/.test(
-        (err as Error).message,
-      );
+      // If initialize() itself failed, the ssh2 Client instance may be in a
+      // broken state (e.g. after a forceful TCP reset during hibernate) and
+      // needs to be recreated before the next attempt.
+      const requiresNewClient =
+        !initializedSuccessfully ||
+        /Instance unusable after fatal error/.test((err as Error).message);
+      const retryableError =
+        requiresNewClient ||
+        /Not connected|Channel open failure|read ECONNRESET|Socket closed/.test(
+          (err as Error).message,
+        );
       this.logger.emit('ssh:failed-forward', {
         host,
         error: String((err as Error).stack),
@@ -173,7 +208,7 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
       if (retryableError) {
         this.connected = false;
         if (retriesLeft > 0) {
-          await this.initialize();
+          await this.initialize(requiresNewClient);
           return await this._connect(req, connectOpts, retriesLeft - 1);
         }
       }

packages/devtools-proxy-support/test/helpers.ts

@@ -39,6 +39,11 @@ export class HTTPServerProxyTestSetup {
   readonly sshServer: SSHServer;
   readonly sshTunnelInfos: TcpipRequestInfo[] = [];
   readonly connections: Duplex[] = [];
+  // Raw TCP sockets accepted by the SSH server, kept so that tests can
+  // forcibly destroy them (simulating the OS dropping connections during
+  // hibernate). These are the underlying net.Sockets, not the high-level
+  // ssh2 Client objects emitted by the SSH server's 'connection' event.
+  readonly sshServerSockets: Socket[] = [];
   canTunnel: () => boolean = () => true;
   authHandler: undefined | ((username: string, password: string) => boolean);
 
@@ -169,6 +174,29 @@ export class HTTPServerProxyTestSetup {
           });
       },
     );
+    // The ssh2 Server's public 'connection' event hands out high-level Client
+    // objects, but to simulate an abrupt network interruption we need the raw
+    // TCP sockets. ssh2 exposes the underlying net.Server as `_srv`, which
+    // emits 'connection' with the raw socket for every accepted connection.
+    (this.sshServer as unknown as { _srv: Server })._srv.on(
+      'connection',
+      (socket: Socket) => {
+        this.sshServerSockets.push(socket);
+        socket.once('close', () => {
+          const index = this.sshServerSockets.indexOf(socket);
+          if (index !== -1) this.sshServerSockets.splice(index, 1);
+        });
+      },
+    );
+  }
+
+  // Forcibly destroys all currently open SSH server-side TCP sockets using a
+  // TCP reset, simulating an abrupt network interruption such as the OS
+  // killing connections when a laptop hibernates.
+  destroySSHConnections(): void {
+    for (const socket of [...this.sshServerSockets]) {
+      if (!socket.destroyed) socket.resetAndDestroy();
+    }
   }
 
   async listen(): Promise<void> {