fix(devtools-proxy-agent): properly forward errors for Socks5 requests (#457)

Author: addaleax

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

@@ -208,6 +208,33 @@ describe('createSocks5Tunnel', function () {
     expect(await response.text()).to.equal('OK /hello');
   });
 
+  it('properly handles forwarding failures', async function () {
+    tunnel = await setupSocks5Tunnel(
+      {
+        useEnvironmentVariableProxies: true,
+        env: {
+          MONGODB_PROXY: `http://foo:bar@127.0.0.1:1`,
+        },
+      },
+      {},
+      'mongodb://'
+    );
+    if (!tunnel) {
+      // regular conditional instead of assertion so that TS can follow it
+      expect.fail('failed to create Socks5 tunnel');
+    }
+
+    try {
+      const fetch = createFetch({
+        proxy: `socks5://@127.0.0.1:${tunnel.config.proxyPort}`,
+      });
+      await fetch('http://example.com/hello');
+      expect.fail('missed exception');
+    } catch (err: any) {
+      expect(err.message).to.include('Socket closed');
+    }
+  });
+
   context('with a non-HTTP target', function () {
     let netServer: Server;
     beforeEach(async function () {

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

@@ -94,8 +94,28 @@ export async function connectThroughAgent({
 }): Promise<Duplex> {
   const channel = await new Promise<Duplex | undefined>((resolve, reject) => {
     const req = createFakeHttpClientRequest(dstAddr, dstPort, overrideProtocol);
-    req.onSocket = (sock) => {
-      if (sock) resolve(sock);
+    req.on('error', reject);
+    const done = (
+      error: Error | null | undefined,
+      sock: Duplex | undefined
+    ) => {
+      req.off('error', reject);
+      if (error) reject(error);
+      else if (sock) resolve(sock);
+      else
+        reject(
+          new Error(
+            'Received neither error object nor socket from agent.createSocket()'
+          )
+        );
+    };
+
+    // err isn't actually optional but not part of the Node.js typings
+    req.onSocket = (
+      sock: Duplex | undefined,
+      err?: Error | null | undefined
+    ) => {
+      done(err, sock);
     };
     agent.createSocket(
       req,
@@ -107,14 +127,7 @@ export async function connectThroughAgent({
         // Ideally, we would always be using this callback for retrieving the `sock`
         // instance. However, agent-base does not call the callback at all if
         // the agent resolved to another agent (as is the case for e.g. `ProxyAgent`).
-        if (err) reject(err);
-        else if (sock) resolve(sock);
-        else
-          reject(
-            new Error(
-              'Received neither error object nor socket from agent.createSocket()'
-            )
-          );
+        done(err, sock);
       }
     );
   });