Skip to content

Commit 3987770

Browse files
authored
revert: pr 719 as dependency change cause unexpected network issue on windows machine (#729)
## Problem aws/aws-toolkit-vscode#8454 aws/aws-toolkit-vscode#8445 aws/aws-toolkit-vscode#8454 dependency change cause unexpected network issue ## Solution Revert <!--- REMINDER: - Read CONTRIBUTING.md first. - Add test coverage for your changes. - Link to related issues/commits. - Testing: how did you test your changes? - Screenshots if applicable --> ## License By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent e344d3a commit 3987770

File tree

8 files changed

+632
-134
lines changed

8 files changed

+632
-134
lines changed

package-lock.json

Lines changed: 559 additions & 32 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

runtimes/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,13 @@
4949
"vscode-languageserver-protocol": "^3.17.5",
5050
"vscode-uri": "^3.1.0",
5151
"win-ca": "^3.5.1",
52-
"winreg": "^1.2.5"
52+
"registry-js": "^1.16.1"
5353
},
5454
"devDependencies": {
5555
"@types/mocha": "^10.0.9",
5656
"@types/mock-fs": "^4.13.4",
5757
"@types/node": "^22.15.17",
5858
"@types/node-forge": "^1.3.11",
59-
"@types/winreg": "^1.2.36",
6059
"assert": "^2.0.0",
6160
"copyfiles": "^2.4.1",
6261
"husky": "^9.1.7",

runtimes/runtimes/standalone.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('standalone', () => {
4747
let chatStub: sinon.SinonStubbedInstance<encryptedChatModule.EncryptedChat> & encryptedChatModule.EncryptedChat
4848
let baseChatStub: sinon.SinonStubbedInstance<baseChatModule.BaseChat> & baseChatModule.BaseChat
4949

50-
it('should initialize without encryption when no key is present', async () => {
50+
it('should initialize without encryption when no key is present', () => {
5151
sinon.stub(authEncryptionModule, 'shouldWaitForEncryptionKey').returns(false)
5252
authStub = stubInterface<authModule.Auth>()
5353
authStub.getCredentialsProvider.returns({
@@ -61,7 +61,7 @@ describe('standalone', () => {
6161
baseChatStub = stubInterface<baseChatModule.BaseChat>()
6262
sinon.stub(baseChatModule, 'BaseChat').returns(baseChatStub)
6363

64-
await standalone(props)
64+
standalone(props)
6565

6666
sinon.assert.calledWithExactly(authModule.Auth as unknown as sinon.SinonStub, stubConnection, lspRouterStub)
6767
sinon.assert.calledWithExactly(
@@ -126,8 +126,8 @@ describe('standalone', () => {
126126
describe('features', () => {
127127
let features: Features
128128

129-
beforeEach(async () => {
130-
await standalone(props)
129+
beforeEach(() => {
130+
standalone(props)
131131
features = stubServer.getCall(0).args[0]
132132
})
133133

runtimes/runtimes/standalone.ts

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ function setupCrashMonitoring(telemetryEmitter?: (metric: MetricEvent) => void)
169169
* @param props.servers The list of servers to initialize and run
170170
* @returns
171171
*/
172-
export const standalone = async (props: RuntimeProps) => {
172+
export const standalone = (props: RuntimeProps) => {
173173
handleVersionArgument(props.version)
174174

175175
const lspConnection = createConnection(ProposedFeatures.all)
@@ -179,42 +179,48 @@ export const standalone = async (props: RuntimeProps) => {
179179

180180
let auth: Auth
181181
let chat: Chat
182-
await initializeAuth()
182+
initializeAuth()
183183

184184
// Initialize Auth service
185-
async function initializeAuth() {
185+
function initializeAuth() {
186186
if (shouldWaitForEncryptionKey()) {
187187
// Before starting the runtime, accept encryption initialization details
188188
// directly from the destination for standalone runtimes.
189189
// Contract: Only read up to (and including) the first newline (\n).
190-
try {
191-
const encryptionDetails: EncryptionInitialization = await readEncryptionDetails(process.stdin)
192-
validateEncryptionDetails(encryptionDetails)
193-
lspConnection.console.info('Runtime: Initializing runtime with encryption')
194-
auth = new Auth(lspConnection, lspRouter, encryptionDetails.key, encryptionDetails.mode)
195-
chat = new EncryptedChat(lspConnection, encryptionDetails.key, encryptionDetails.mode)
196-
await initializeRuntime(encryptionDetails.key)
197-
} catch (error) {
198-
console.error(error)
199-
// arbitrary 5 second timeout to ensure console.error flushes before process exit
200-
// note: webpacked version may output exclusively to stdout, not stderr.
201-
setTimeout(() => {
202-
process.exit(10)
203-
}, 5000)
204-
}
190+
readEncryptionDetails(process.stdin)
191+
.then(
192+
(encryptionDetails: EncryptionInitialization) => {
193+
validateEncryptionDetails(encryptionDetails)
194+
lspConnection.console.info('Runtime: Initializing runtime with encryption')
195+
auth = new Auth(lspConnection, lspRouter, encryptionDetails.key, encryptionDetails.mode)
196+
chat = new EncryptedChat(lspConnection, encryptionDetails.key, encryptionDetails.mode)
197+
initializeRuntime(encryptionDetails.key)
198+
},
199+
error => {
200+
console.error(error)
201+
// arbitrary 5 second timeout to ensure console.error flushes before process exit
202+
// note: webpacked version may output exclusively to stdout, not stderr.
203+
setTimeout(() => {
204+
process.exit(10)
205+
}, 5000)
206+
}
207+
)
208+
.catch((error: Error) => {
209+
console.error('Error at runtime initialization:', error.message)
210+
})
205211
} else {
206212
lspConnection.console.info('Runtime: Initializing runtime without encryption')
207213
auth = new Auth(lspConnection, lspRouter)
208214

209-
await initializeRuntime()
215+
initializeRuntime()
210216
}
211217
}
212218

213219
// Initialize the LSP connection based on the supported LSP capabilities
214220
// TODO: make this dependent on the actual requirements of the
215221
// capabilities parameter.
216222

217-
async function initializeRuntime(encryptionKey?: string) {
223+
function initializeRuntime(encryptionKey?: string) {
218224
const documents = new TextDocuments(TextDocument)
219225
// Set up telemetry over LSP
220226
const telemetry: Telemetry = {
@@ -367,8 +373,6 @@ export const standalone = async (props: RuntimeProps) => {
367373

368374
const agent = newAgent()
369375

370-
const v3ProxyConfig = await sdkProxyConfigManager.getV3ProxyConfig()
371-
372376
// Initialize every Server
373377
const disposables = props.servers.map(s => {
374378
// Create LSP server representation that holds internal server state
@@ -455,7 +459,9 @@ export const standalone = async (props: RuntimeProps) => {
455459
current_config: P
456460
): T => {
457461
try {
458-
const requestHandler = isExperimentalProxy ? v3ProxyConfig : makeProxyConfigv3Standalone(workspace)
462+
const requestHandler = isExperimentalProxy
463+
? sdkProxyConfigManager.getV3ProxyConfig()
464+
: makeProxyConfigv3Standalone(workspace)
459465

460466
logging.log(`Using ${isExperimentalProxy ? 'experimental' : 'standard'} proxy util`)
461467

runtimes/runtimes/util/standalone/experimentalProxyUtil.test.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ describe('ProxyConfigManager', function () {
5353

5454
beforeEach(() => {
5555
originalEnv = { ...process.env }
56+
process.env = {}
5657

5758
telemetryStub = {
5859
emitMetric: sinon.stub(),
@@ -71,16 +72,16 @@ describe('ProxyConfigManager', function () {
7172
mockFs.restore()
7273
})
7374

74-
it('should cache and return same V3 config', async () => {
75-
const config1 = await proxyManager.getV3ProxyConfig()
76-
const config2 = await proxyManager.getV3ProxyConfig()
75+
it('should cache and return same V3 config', () => {
76+
const config1 = proxyManager.getV3ProxyConfig()
77+
const config2 = proxyManager.getV3ProxyConfig()
7778

7879
assert.strictEqual(config1, config2)
7980
})
8081

81-
it('should use secure agent for V3 config', async () => {
82+
it('should use secure agent for V3 config', () => {
8283
const getAgentSpy = sinon.spy(proxyManager, 'getSecureAgent')
83-
await proxyManager.getV3ProxyConfig()
84+
proxyManager.getV3ProxyConfig()
8485

8586
assert(getAgentSpy.calledOnce)
8687
})
@@ -263,8 +264,8 @@ describe('ProxyConfigManager', function () {
263264
readLinuxCertificatesStub.returns(sysCerts)
264265
})
265266

266-
it('should create HttpsAgent when no proxy set', async () => {
267-
const agent = await proxyManager.createSecureAgent()
267+
it('should create HttpsAgent when no proxy set', () => {
268+
const agent = proxyManager.createSecureAgent()
268269

269270
assert(agent instanceof HttpsAgent)
270271
assert.strictEqual((agent as HttpsAgent).options.rejectUnauthorized, true)
@@ -283,9 +284,9 @@ describe('ProxyConfigManager', function () {
283284
)
284285
})
285286

286-
it('should create HttpsProxyAgent when proxy set', async () => {
287+
it('should create HttpsProxyAgent when proxy set', () => {
287288
process.env.HTTPS_PROXY = 'https://proxy'
288-
const agent = await proxyManager.createSecureAgent()
289+
const agent = proxyManager.createSecureAgent()
289290

290291
assert(agent instanceof HttpsProxyAgent)
291292
assert.strictEqual(agent.options.rejectUnauthorized, true)

runtimes/runtimes/util/standalone/experimentalProxyUtil.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,16 @@ export class ProxyConfigManager {
2525

2626
constructor(private readonly telemetry: Telemetry) {}
2727

28-
async getSecureAgent(): Promise<HttpsAgent | HttpsProxyAgent> {
28+
getSecureAgent(): HttpsAgent | HttpsProxyAgent {
2929
if (!this.cachedAgent) {
30-
this.cachedAgent = await this.createSecureAgent()
30+
this.cachedAgent = this.createSecureAgent()
3131
}
3232
return this.cachedAgent
3333
}
3434

35-
public async getV3ProxyConfig(): Promise<NodeHttpHandler> {
35+
public getV3ProxyConfig(): NodeHttpHandler {
3636
if (!this.cachedV3Config) {
37-
const agent = await this.getSecureAgent()
37+
const agent = this.getSecureAgent()
3838
this.cachedV3Config = new NodeHttpHandler({
3939
httpAgent: agent,
4040
httpsAgent: agent,
@@ -57,12 +57,12 @@ export class ProxyConfigManager {
5757
return undefined
5858
}
5959

60-
private static async getSystemProxy(): Promise<string | undefined> {
60+
private static getSystemProxy(): string | undefined {
6161
switch (process.platform) {
6262
case 'darwin':
6363
return getMacSystemProxy()?.proxyUrl
6464
case 'win32':
65-
return (await getWindowsSystemProxy())?.proxyUrl
65+
return getWindowsSystemProxy()?.proxyUrl
6666
default:
6767
return undefined
6868
}
@@ -166,7 +166,7 @@ export class ProxyConfigManager {
166166
*
167167
* @returns {HttpsAgent | HttpsProxyAgent}
168168
*/
169-
async createSecureAgent(): Promise<HttpsAgent | HttpsProxyAgent> {
169+
createSecureAgent(): HttpsAgent | HttpsProxyAgent {
170170
const certs = this.getCertificates()
171171
const agentOptions = {
172172
ca: certs,
@@ -184,7 +184,7 @@ export class ProxyConfigManager {
184184
}
185185

186186
// Fall back to OS auto‑detect (HTTP or HTTPS only)
187-
const sysProxyUrl = await ProxyConfigManager.getSystemProxy()
187+
const sysProxyUrl = ProxyConfigManager.getSystemProxy()
188188
if (sysProxyUrl) {
189189
this.emitProxyMetric('AutoDetect', certs.length, sysProxyUrl)
190190
return new HttpsProxyAgent({ ...agentOptions, proxy: sysProxyUrl })

runtimes/runtimes/util/standalone/getProxySettings/getWindowsProxySettings.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import * as assert from 'assert'
66
import { getWindowsSystemProxy } from './getWindowsProxySettings'
77

88
describe('getWindowsSystemProxy', () => {
9-
it('can get the Windows system proxy', async function () {
9+
it('can get the Windows system proxy', function () {
1010
if (process.platform !== 'win32') return this.skip()
1111

12-
const result = await getWindowsSystemProxy()
12+
const result = getWindowsSystemProxy()
1313
assert.ok(result === undefined || (typeof result === 'object' && result !== null))
1414

1515
if (result) {
@@ -18,15 +18,15 @@ describe('getWindowsSystemProxy', () => {
1818
}
1919
})
2020

21-
it('returns undefined on non-Windows platforms', async function () {
21+
it('returns undefined on non-Windows platforms', function () {
2222
if (process.platform === 'win32') return this.skip()
2323

24-
const result = await getWindowsSystemProxy()
24+
const result = getWindowsSystemProxy()
2525
assert.strictEqual(result, undefined)
2626
})
2727

28-
it('handles registry access failure gracefully', async function () {
28+
it('handles registry access failure gracefully', function () {
2929
// This test verifies the function doesn't throw
30-
assert.doesNotThrow(async () => await getWindowsSystemProxy())
30+
assert.doesNotThrow(() => getWindowsSystemProxy())
3131
})
3232
})

runtimes/runtimes/util/standalone/getProxySettings/getWindowsProxySettings.ts

Lines changed: 15 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,73 +5,36 @@
55
* Based on windows-system-proxy 1.0.0 (Apache-2.0). Modified for synchronous use
66
* https://github.com/httptoolkit/windows-system-proxy/blob/main/src/index.ts
77
*/
8-
import winreg from 'winreg'
8+
import { enumerateValues, HKEY, RegistryValue } from 'registry-js'
99

1010
export interface ProxyConfig {
1111
proxyUrl: string
1212
noProxy: string[]
1313
}
1414

15-
const KEY_PROXY_ENABLE = 'ProxyEnable'
16-
const KEY_PROXY_SERVER = 'ProxyServer'
17-
const KEY_PROXY_OVERRIDE = 'ProxyOverride'
18-
19-
type WindowsProxyRegistryKeys = {
20-
proxyEnable: string | undefined
21-
proxyServer: string | undefined
22-
proxyOverride: string | undefined
23-
}
24-
25-
function readWindowsRegistry(): Promise<WindowsProxyRegistryKeys> {
26-
return new Promise((resolve, reject) => {
27-
const regKey = new winreg({
28-
hive: winreg.HKCU,
29-
key: '\\Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings',
30-
})
31-
32-
regKey.values((err: Error, items: winreg.RegistryItem[]) => {
33-
if (err) {
34-
console.warn('', err.message)
35-
resolve({
36-
proxyEnable: undefined,
37-
proxyServer: undefined,
38-
proxyOverride: undefined,
39-
})
40-
return
41-
}
42-
43-
const results: Record<string, string> = {}
44-
45-
items.forEach((item: winreg.RegistryItem) => {
46-
results[item.name] = item.value as string
47-
})
48-
49-
resolve({
50-
proxyEnable: results[KEY_PROXY_ENABLE],
51-
proxyServer: results[KEY_PROXY_SERVER],
52-
proxyOverride: results[KEY_PROXY_OVERRIDE],
53-
})
54-
})
55-
})
56-
}
15+
export function getWindowsSystemProxy(): ProxyConfig | undefined {
16+
const proxyValues = enumerateValues(
17+
HKEY.HKEY_CURRENT_USER,
18+
'Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings'
19+
)
20+
console.debug(`Retrieved ${proxyValues.length} registry values for proxy settings`)
5721

58-
export async function getWindowsSystemProxy(): Promise<ProxyConfig | undefined> {
59-
const registryValues = await readWindowsRegistry()
60-
const proxyEnabled = registryValues.proxyEnable
61-
const proxyServer = registryValues.proxyServer
62-
const proxyOverride = registryValues.proxyOverride
22+
const proxyEnabled = getValue(proxyValues, 'ProxyEnable')
23+
const proxyServer = getValue(proxyValues, 'ProxyServer')
6324

64-
if (!proxyEnabled || !proxyServer) {
25+
if (!proxyEnabled || !proxyEnabled.data || !proxyServer || !proxyServer.data) {
6526
console.debug('Proxy not enabled or server not configured')
6627
return undefined
6728
}
6829

30+
// Build noProxy list from ProxyOverride (semicolon-separated, with <local> → localhost,127.0.0.1,::1)
31+
const proxyOverride = getValue(proxyValues, 'ProxyOverride')?.data
6932
const noProxy = (proxyOverride ? (proxyOverride as string).split(';') : []).flatMap(host =>
7033
host === '<local>' ? ['localhost', '127.0.0.1', '::1'] : [host]
7134
)
7235

7336
// Parse proxy configuration which can be in multiple formats
74-
const proxyConfigString = proxyServer
37+
const proxyConfigString = proxyServer.data as string
7538

7639
if (proxyConfigString.startsWith('http://') || proxyConfigString.startsWith('https://')) {
7740
console.debug('Using full URL format proxy configuration')
@@ -115,3 +78,5 @@ export async function getWindowsSystemProxy(): Promise<ProxyConfig | undefined>
11578
}
11679
}
11780
}
81+
82+
const getValue = (values: readonly RegistryValue[], name: string) => values.find(value => value?.name === name)

0 commit comments

Comments
 (0)