From 42f4fe7d3853527ec611cc04c7e928a74433c1ff Mon Sep 17 00:00:00 2001
From: Rik <rik.nauta@sourcegraph.com>
Date: Tue, 22 Oct 2024 22:25:08 +0200
Subject: [PATCH] Networking: Fix leftover PR comments (#5916)

There were a few leftover undressed/reverted PR comments on
https://github.com/sourcegraph/cody/pull/5883

Now we:

- No longer throw an exception on early agent access. The previous
exception was only used during debugging of an import order issue.
- Fixed mentioned function comments / typos

## Test plan
- No functionality should have changed, existing tests pass.
---
 lib/shared/src/configuration/environment.ts | 21 +++++++++++----------
 lib/shared/src/fetch.patch.ts               | 21 +++++++--------------
 vscode/src/net/net.patch.ts                 |  4 +---
 3 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/lib/shared/src/configuration/environment.ts b/lib/shared/src/configuration/environment.ts
index a5fb47f793a8..7b34c606e39c 100644
--- a/lib/shared/src/configuration/environment.ts
+++ b/lib/shared/src/configuration/environment.ts
@@ -21,10 +21,10 @@ import type { UIKind } from 'vscode' // types are ok
 
 export const cenv = defineEnvBuilder({
     /**
-     * A combination of HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment
-     * variables for easier consumption.
+     * A proxy string that falls back to Node's HTTP_PROXY and HTTPS_PROXY if
+     * not explicitly configured (or disabled with 'false').
      */
-    CODY_NODE_DEFAULT_PROXY: codyProxy,
+    CODY_NODE_DEFAULT_PROXY: proxyStringWithNodeFallback,
 
     /**
      * Enables unstable internal testing configuration to be read from settings.json
@@ -161,13 +161,14 @@ function uiKind(uiKind: string | undefined): UIKind | undefined {
     }
 }
 
-function codyProxy(envValue: string | undefined): string | undefined {
-    switch (bool(envValue)) {
-        case false:
-            // explicitly disabled
-            return undefined
-        default:
-            break
+/**
+ * If explicitly set to false no proxy value is returned. Or returns the string
+ * value set otherwise. If no explicit value is set value defaults to the
+ * default HTTPS_PROXY or HTTP_PROXY values (in that order).
+ */
+function proxyStringWithNodeFallback(envValue: string | undefined): string | undefined {
+    if (bool(envValue) === false) {
+        return undefined
     }
 
     const forcedProxy = str(envValue)
diff --git a/lib/shared/src/fetch.patch.ts b/lib/shared/src/fetch.patch.ts
index e1818551a6bf..f5cc4ada71e5 100644
--- a/lib/shared/src/fetch.patch.ts
+++ b/lib/shared/src/fetch.patch.ts
@@ -2,14 +2,15 @@
  * In node environments, it might be necessary to set up a custom agent to
  * control the network requests being made.
  *
- * To do this, we have a mutable agent variable that can be set externally. For
- * this to work we need to ensure no other dependencies have loaded yet. That's
- * why this module is seperate from the fetch.ts file which simply re-exports
- * this variable. This way the patching code can simply import this specific
- * file and gain early access to the agentRef
+ * To do this, we expose the globalAgentRef variable that can be mutated
+ * externally.
  *
  * 🚨 Do not import this file unless you're specifically patching the variable.
- * For read-only access simply import the fetch.ts file
+ * For read-only access simply import the fetch.ts file. This variable was only
+ * isolated into this separate .patch file so that patching code can import this
+ * without bringing in transitive dependencies.
+ *
+ * An example of such a patch where this is critical is in `vscode/src/net/net.patch.ts`
  */
 
 import type EventEmitter from 'node:events'
@@ -19,12 +20,8 @@ import type { Agent } from 'agent-base'
 
 let _globalAgent: Agent | undefined
 let _eventEmitter: EventEmitter<NetEventMap> | undefined
-let _blockEarlyAccess = false
 export const globalAgentRef = {
     get agent() {
-        if (_blockEarlyAccess && !_globalAgent) {
-            throw new Error('Agent was used before it was initialized')
-        }
         return _globalAgent
     },
 
@@ -41,10 +38,6 @@ export const globalAgentRef = {
         _eventEmitter = v
     },
 
-    set blockEarlyAccess(v: boolean) {
-        _blockEarlyAccess = v
-    },
-
     get isSet() {
         return _globalAgent !== undefined
     },
diff --git a/vscode/src/net/net.patch.ts b/vscode/src/net/net.patch.ts
index 2a6b013f8831..89a1dee68445 100644
--- a/vscode/src/net/net.patch.ts
+++ b/vscode/src/net/net.patch.ts
@@ -1,4 +1,4 @@
-// 🚨 This patch file mast have NO additional dependencies as import order
+// 🚨 This patch file must have NO additional dependencies as import order
 // changes might cause modules to load before the networking is patched.
 import EventEmitter from 'node:events'
 import type * as http from 'node:http'
@@ -11,8 +11,6 @@ import type * as internalVSCodeAgent from './vscode-network-proxy'
 export const bypassVSCodeSymbol = Symbol('bypassVSCodeSymbol')
 let patched = false
 patchNetworkStack()
-// This needs to happen after so that we don't break import order
-globalAgentRef.blockEarlyAccess = true
 
 function patchNetworkStack(): void {
     if (patched) {