Merge pull request #2487 from cdr/proxy-docs-86d4

proxy-agent: Use proxy-from-env and add docs

src/node/proxy_agent.ts

@@ -1,12 +1,15 @@
 import { logger } from "@coder/logger"
 import * as http from "http"
-import * as proxyagent from "proxy-agent"
+import * as proxyAgent from "proxy-agent"
+import * as proxyFromEnv from "proxy-from-env"
 
 /**
- * This file does not have anything to do with the code-server proxy.
- * It's for $HTTP_PROXY support!
+ * This file has nothing to do with the code-server proxy.
+ * It is to support $HTTP_PROXY, $HTTPS_PROXY and $NO_PROXY.
+ *
  * - https://github.com/cdr/code-server/issues/124
  * - https://www.npmjs.com/package/proxy-agent
+ * - https://www.npmjs.com/package/proxy-from-env
  *
  * This file exists in two locations:
  * - src/node/proxy_agent.ts
@@ -15,48 +18,64 @@ import * as proxyagent from "proxy-agent"
  */
 
 /**
- * monkeyPatch patches the node HTTP/HTTPS library to route all requests through our
- * custom agent from the proxyAgent package.
+ * monkeyPatch patches the node http,https modules to route all requests through the
+ * agent we get from the proxy-agent package.
+ *
+ * This approach only works if there is no code specifying an explicit agent when making
+ * a request.
+ *
+ * None of our code ever passes in a explicit agent to the http,https modules.
+ * VS Code's does sometimes but only when a user sets the http.proxy configuration.
+ * See https://code.visualstudio.com/docs/setup/network#_legacy-proxy-server-support
+ *
+ * Even if they do, it's probably the same proxy so we should be fine! And those knobs
+ * are deprecated anyway.
  */
-export function monkeyPatch(vscode: boolean): void {
-  // We do not support HTTPS_PROXY here to avoid confusion. proxy-agent will automatically
-  // use the correct protocol to connect to the proxy depending on the requested URL.
-  //
-  // We could implement support ourselves to allow people to configure the proxy used for
-  // HTTPS vs HTTP but there doesn't seem to be much value in that.
-  //
-  // At least of right now, it'd just be plain confusing to support HTTPS_PROXY when proxy-agent
-  // will still use HTTP to hit it for HTTP requests.
-  const proxyURL = process.env.HTTP_PROXY || process.env.http_proxy
-  if (!proxyURL) {
-    return
+export function monkeyPatch(inVSCode: boolean): void {
+  if (shouldEnableProxy()) {
+    const http = require("http")
+    const https = require("https")
+
+    // If we do not pass in a proxy URL, proxy-agent will get the URL from the environment.
+    // See https://www.npmjs.com/package/proxy-from-env.
+    // Also see shouldEnableProxy.
+    const pa = newProxyAgent(inVSCode)
+    http.globalAgent = pa
+    https.globalAgent = pa
   }
+}
 
-  logger.debug(`using $HTTP_PROXY ${process.env.HTTP_PROXY}`)
-
-  let pa: http.Agent
+function newProxyAgent(inVSCode: boolean): http.Agent {
   // The reasoning for this split is that VS Code's build process does not have
   // esModuleInterop enabled but the code-server one does. As a result depending on where
   // we execute, we either have a default attribute or we don't.
   //
   // I can't enable esModuleInterop in VS Code's build process as it breaks and spits out
-  // a huge number of errors.
-  if (vscode) {
-    pa = new (proxyagent as any)(process.env.HTTP_PROXY)
+  // a huge number of errors. And we can't use require as otherwise the modules won't be
+  // included in the final product.
+  if (inVSCode) {
+    return new (proxyAgent as any)()
   } else {
-    pa = new (proxyagent as any).default(process.env.HTTP_PROXY)
+    return new (proxyAgent as any).default()
+  }
+}
+
+// If they have $NO_PROXY set to example.com then this check won't work!
+// But that's drastically unlikely.
+function shouldEnableProxy(): boolean {
+  let shouldEnable = false
+
+  const httpProxy = proxyFromEnv.getProxyForUrl(`http://example.com`)
+  if (httpProxy) {
+    shouldEnable = true
+    logger.debug(`using $HTTP_PROXY ${httpProxy}`)
   }
 
-  /**
-   * None of our code ever passes in a explicit agent to the http modules but VS Code's
-   * does sometimes but only when a user sets the http.proxy configuration.
-   * See https://code.visualstudio.com/docs/setup/network#_legacy-proxy-server-support
-   *
-   * Even if they do, it's probably the same proxy so we should be fine! And those are
-   * deprecated anyway.
-   */
-  const http = require("http")
-  const https = require("https")
-  http.globalAgent = pa
-  https.globalAgent = pa
+  const httpsProxy = proxyFromEnv.getProxyForUrl(`https://example.com`)
+  if (httpsProxy) {
+    shouldEnable = true
+    logger.debug(`using $HTTPS_PROXY ${httpsProxy}`)
+  }
+
+  return shouldEnable
 }