Skip to content

Commit

Permalink
Networking: Fix leftover PR comments (#5916)
Browse files Browse the repository at this point in the history
There were a few leftover undressed/reverted PR comments on
#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.
  • Loading branch information
RXminuS authored Oct 22, 2024
1 parent e44aa42 commit 42f4fe7
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 27 deletions.
21 changes: 11 additions & 10 deletions lib/shared/src/configuration/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 7 additions & 14 deletions lib/shared/src/fetch.patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
},

Expand All @@ -41,10 +38,6 @@ export const globalAgentRef = {
_eventEmitter = v
},

set blockEarlyAccess(v: boolean) {
_blockEarlyAccess = v
},

get isSet() {
return _globalAgent !== undefined
},
Expand Down
4 changes: 1 addition & 3 deletions vscode/src/net/net.patch.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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) {
Expand Down

0 comments on commit 42f4fe7

Please sign in to comment.