WIP

Author: addaleax

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

@@ -14,15 +14,16 @@ describe('createAgent', function () {
     url: string,
     agent: Agent
   ): Promise<IncomingMessage & { body: string }> => {
-    const getFn = new URL(url).protocol === 'https:' ? httpsGet : httpGet;
+    const nodeJSBuiltinGet =
+      new URL(url).protocol === 'https:' ? httpsGet : httpGet;
     const options = {
       agent,
       ca: setup.tlsOptions.ca,
       checkServerIdentity: () => undefined, // allow hostname mismatches
     };
     agents.push(agent);
     const res = await new Promise<IncomingMessage>((resolve, reject) =>
-      getFn(url, options, resolve).once('error', reject)
+      nodeJSBuiltinGet(url, options, resolve).once('error', reject)
     );
     let body = '';
     res.setEncoding('utf8');

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

@@ -10,6 +10,9 @@ import { SSHAgent } from './ssh';
 import type { ProxyLogEmitter } from './logging';
 import type { EventEmitter } from 'events';
 
+// Helper type that represents an https.Agent (= connection factory)
+// with some custom properties that TS does not know about and/or
+// that we add for our own purposes.
 export type AgentWithInitialize = Agent & {
   // This is genuinely custom for our usage (to allow establishing an SSH tunnel
   // first before starting to push connections through it)
@@ -24,7 +27,7 @@ export type AgentWithInitialize = Agent & {
     cb: (err: Error | null, s?: Duplex) => void
   ): void;
 
-  // http.Agent is an EventEmitter
+  // http.Agent is an EventEmitter, just missing from @types/node
 } & Partial<EventEmitter>;
 
 export function createAgent(

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

@@ -6,6 +6,8 @@ import type { DevtoolsProxyOptions } from './proxy-options';
 
 declare const __webpack_require__: unknown;
 
+// The original version of this code was largely taken from
+// https://github.com/mongodb-js/mongosh/blob/8e6962432397154941f593c847d8f774bfd49f1c/packages/import-node-fetch/src/index.ts
 async function importNodeFetch(): Promise<typeof fetch> {
   // Node-fetch is an ESM module from 3.x
   // Importing ESM modules to CommonJS is possible with a dynamic import.

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

@@ -1,3 +1,8 @@
+// Most mongoLogId() calls here come from code that was
+// previously part of the MongoDB Compass monorepo, hence the specific
+// values used here; in particular,
+// https://github.com/mongodb-js/compass/tree/55a5a608713d7316d158dc66febeb6b114d8b40d/packages/ssh-tunnel/src
+
 interface BaseSocks5RequestMetadata {
   srcAddr: string;
   srcPort: number;

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

@@ -18,7 +18,7 @@ import { HTTPServerProxyTestSetup } from '../test/helpers';
 
 describe('proxy options handling', function () {
   describe('proxyConfForEnvVars', function () {
-    it('should return a map of proxies and noProxy', function () {
+    it('should return a map listing configured proxies and no-proxy hosts', function () {
       const env = {
         HTTP_PROXY: 'http://proxy.example.com',
         HTTPS_PROXY: 'https://proxy.example.com',
@@ -145,7 +145,12 @@ describe('proxy options handling', function () {
         ).to.deep.equal({});
       });
     });
+
     context('integration tests', function () {
+      // These are some integration tests that leverage the fact that
+      // Electron can be used as a script runner to verify that our generated
+      // Electron proxyh configuration actually behaves the way it is intended to.
+
       let childProcess: ChildProcess;
       let exitPromise: Promise<unknown>;
       let server: Server;
@@ -164,6 +169,8 @@ describe('proxy options handling', function () {
         setup = new HTTPServerProxyTestSetup();
         await setup.listen();
 
+        // Use a TCP connection for communication with the electron process;
+        // see electron-test-server.js for details.
         server = createServer();
         server.listen(0);
         await once(server, 'listening');
@@ -185,7 +192,7 @@ describe('proxy options handling', function () {
           }
         );
         exitPromise = once(childProcess, 'exit').catch(() => {
-          /* ignore */
+          // ignore unhandledRejection warning/error
         });
         await once(childProcess, 'spawn');
 
@@ -205,6 +212,8 @@ describe('proxy options handling', function () {
         };
 
         testResolveProxy = async (proxyOptions, url) => {
+          // https://www.electronjs.org/docs/latest/api/app#appsetproxyconfig
+          // https://www.electronjs.org/docs/latest/api/app#appresolveproxyurl
           return await runJS(`app.setProxy(${JSON.stringify(
             translateToElectronProxyConfig(proxyOptions)
           )}).then(_ => {

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

@@ -35,6 +35,7 @@ interface ElectronProxyConfig {
   proxyBypassRules?: string;
 }
 
+// Reads through all environment variables and gathers proxy settings from them
 export function proxyConfForEnvVars(env: Record<string, string | undefined>): {
   map: Map<string, string>;
   noProxy: string;
@@ -52,6 +53,7 @@ export function proxyConfForEnvVars(env: Record<string, string | undefined>): {
   return { map, noProxy };
 }
 
+// Whether a given URL should be proxied or not, according to `noProxy`
 function shouldProxy(noProxy: string, url: URL): boolean {
   if (!noProxy) return true;
   if (noProxy === '*') return false;
@@ -72,6 +74,8 @@ function shouldProxy(noProxy: string, url: URL): boolean {
   return true;
 }
 
+// Create a function which returns the proxy URL for a given target URL,
+// based on the proxy config passed to it.
 export function proxyForUrl(
   proxyOptions: DevtoolsProxyOptions
 ): (url: string) => string {
@@ -109,6 +113,17 @@ export function proxyForUrl(
 
 function validateElectronProxyURL(url: URL | string): string {
   url = new URL(url.toString());
+  // ssh and authenticated proxies are not supported by Electron/Chromium currently.
+  // (See https://issues.chromium.org/issues/40829748, https://bugs.chromium.org/p/chromium/issues/detail?id=1309413
+  // and related tickets for history.)
+  // If we do want to support them at some point, a possible way to achieve this would be
+  // to use a local in-application Socks5 proxy server which reaches out to the
+  // actual target proxy (or directly connects, depending on the configuration),
+  // and then passing the local proxy's information to Electron.
+  // A core downside with this approach, however, is that because the proxy cannot be
+  // authenticated, it would be available to any local application, which is problematic
+  // when running on multi-user machine where the proxy would then be available to
+  // arbitraty users.
   if (url.protocol === 'ssh:') {
     throw new Error(
       `Using ssh:// proxies for generic browser proxy usage is not supported (translating '${redactUrl(
@@ -150,11 +165,13 @@ function validateElectronProxyURL(url: URL | string): string {
   return url.toString().replace(/\/$/, '');
 }
 
+// Convert our own `DevtoolsProxyOptions` to the format used by Electron.
 export function translateToElectronProxyConfig(
   proxyOptions: DevtoolsProxyOptions
 ): ElectronProxyConfig {
   if (proxyOptions.proxy) {
     let url = proxyOptions.proxy;
+    // https://en.wikipedia.org/wiki/Proxy_auto-config
     if (new URL(url).protocol.startsWith('pac+')) {
       url = url.replace('pac+', '');
       return {

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

@@ -61,6 +61,8 @@ function createFakeHttpClientRequest(dstAddr: string, dstPort: number) {
   });
 }
 
+// The original version of this code was largely taken from
+// https://github.com/mongodb-js/compass/tree/55a5a608713d7316d158dc66febeb6b114d8b40d/packages/ssh-tunnel/src
 class Socks5Server extends EventEmitter implements Tunnel {
   public logger: ProxyLogEmitter = new EventEmitter();
   private readonly agent: AgentWithInitialize;

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

@@ -13,6 +13,8 @@ import type { ProxyLogEmitter } from './logging';
 import { connect as tlsConnect } from 'tls';
 import type { Socket } from 'net';
 
+// The original version of this code was largely taken from
+// https://github.com/mongodb-js/compass/tree/55a5a608713d7316d158dc66febeb6b114d8b40d/packages/ssh-tunnel/src
 export class SSHAgent extends AgentBase implements AgentWithInitialize {
   public logger: ProxyLogEmitter;
   private readonly proxyOptions: Readonly<DevtoolsProxyOptions>;
@@ -31,7 +33,7 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize {
   constructor(options: DevtoolsProxyOptions, logger?: ProxyLogEmitter) {
     super();
     (this as AgentWithInitialize).on?.('error', () => {
-      //Errors should not crash the process
+      // Errors here should not crash the process
     });
     this.logger = logger ?? new EventEmitter();
     this.proxyOptions = options;

packages/devtools-proxy-support/test/electron-test-server.js

@@ -2,6 +2,11 @@
 const { app } = require('electron');
 const { once } = require('events');
 const { connect } = require('net');
+
+// Essentially a REPL that we use for running Electron code.
+// Communicates with a parent process via TCP and runs the code
+// it receives over that socket. Input and output are NUL-delimited
+// chunks of UTF-8 strings.
 (async function () {
   try {
     await app.whenReady();
@@ -14,8 +19,13 @@ const { connect } = require('net');
       while (buffer.includes('\0')) {
         const readyToExecute = buffer.substring(0, buffer.indexOf('\0'));
         buffer = buffer.substring(buffer.indexOf('\0') + 1);
-        const result = JSON.stringify(await eval(JSON.parse(readyToExecute)));
-        socket.write(result + '\0');
+        let result;
+        try {
+          result = await eval(JSON.parse(readyToExecute));
+        } catch (err) {
+          result = { message: err.message, stack: err.stack, ...err };
+        }
+        socket.write(JSON.stringify(result) + '\0');
       }
     }
     process.exit(0);