Skip to content

Commit 665e82e

Browse files
nagilsonCopilot
andcommitted
Fix isOnline() to fall back to HTTP HEAD when DNS fails behind proxy
In enterprise proxy environments, the client machine may not resolve external DNS directly — DNS resolution is handled by the proxy server. The existing DNS-only check in isOnline() would incorrectly report the machine as offline, causing unnecessary fallback to cached/offline installs. When DNS resolution fails and the axios client is available, isOnline() now attempts a lightweight HEAD request to www.microsoft.com through the auto-detected (or manually configured) proxy. This ensures connectivity is correctly detected in proxy environments while keeping DNS as the fast-path for non-proxy setups. Refactors GetProxyAgentIfNeeded by extracting proxy resolution into a standalone getProxyAgent method that does not depend on IAcquisitionWorkerContext. This allows isOnline and other callers without a full context to reuse the same proxy detection logic without duplication. Fixes #2594 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6805569 commit 665e82e

3 files changed

Lines changed: 212 additions & 12 deletions

File tree

vscode-dotnet-runtime-library/src/Utils/WebRequestWorkerSingleton.ts

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export class WebRequestWorkerSingleton
281281
// ... 100 ms is there as a default to prevent the dns resolver from throwing a runtime error if the user sets timeoutSeconds to 0.
282282

283283
const dnsResolver = new dns.promises.Resolver({ timeout: expectedDNSResolutionTimeMs });
284-
const couldConnect = await dnsResolver.resolve(microsoftServerHostName).then(() =>
284+
const dnsOnline = await dnsResolver.resolve(microsoftServerHostName).then(() =>
285285
{
286286
return true;
287287
}).catch((error: any) =>
@@ -290,7 +290,47 @@ export class WebRequestWorkerSingleton
290290
return false;
291291
});
292292

293-
return couldConnect;
293+
if (dnsOnline)
294+
{
295+
return true;
296+
}
297+
298+
// DNS failed — but in proxy environments, the proxy handles DNS resolution, so direct DNS lookups may fail
299+
// even though HTTP connectivity works fine. Fall back to a lightweight HEAD request through the proxy-configured client.
300+
if (this.client)
301+
{
302+
const httpFallbackTimeoutMs = Math.max(timeoutSec * 1000, 2000);
303+
const proxyAgent = await this.getProxyAgent(undefined, eventStream);
304+
305+
const headOptions: object = {
306+
timeout: httpFallbackTimeoutMs,
307+
cache: false,
308+
validateStatus: () => true, // Any HTTP response means we're online, even 4xx/5xx
309+
...(proxyAgent !== null && { proxy: false }),
310+
...(proxyAgent !== null && { httpsAgent: proxyAgent }),
311+
};
312+
313+
const headOnline = await this.client.head(`https://${microsoftServerHostName}`, headOptions)
314+
.then(() =>
315+
{
316+
return true; // Any response at all means we have connectivity
317+
})
318+
.catch(() =>
319+
{
320+
return false;
321+
});
322+
323+
if (headOnline)
324+
{
325+
eventStream.post(new OfflineDetectionLogicTriggered(new EventCancellationError('DnsFailedButHttpSucceeded',
326+
`DNS resolution failed but HTTP HEAD request succeeded. This may indicate a proxy is handling DNS.`),
327+
`DNS failed but HTTP connectivity confirmed via HEAD request to ${microsoftServerHostName}.`));
328+
}
329+
330+
return headOnline;
331+
}
332+
333+
return false;
294334
}
295335
/**
296336
*
@@ -321,11 +361,22 @@ export class WebRequestWorkerSingleton
321361
}
322362

323363
private async GetProxyAgentIfNeeded(ctx: IAcquisitionWorkerContext): Promise<HttpsProxyAgent<string> | null>
364+
{
365+
return this.getProxyAgent(ctx.proxyUrl, ctx.eventStream);
366+
}
367+
368+
/**
369+
* Resolves a proxy agent from the manual proxy URL or auto-detected system proxy settings.
370+
* Decoupled from IAcquisitionWorkerContext so it can be used by isOnline and other callers that don't have a full context.
371+
*/
372+
private async getProxyAgent(manualProxyUrl?: string, eventStream?: IEventStream): Promise<HttpsProxyAgent<string> | null>
324373
{
325374
try
326375
{
376+
const hasManualProxy = manualProxyUrl ? manualProxyUrl !== '""' : false;
377+
327378
let discoveredProxy = '';
328-
if (!this.proxySettingConfiguredManually(ctx))
379+
if (!hasManualProxy)
329380
{
330381
const autoDetectProxies = await getProxySettings();
331382
if (autoDetectProxies?.https)
@@ -338,17 +389,17 @@ export class WebRequestWorkerSingleton
338389
}
339390
}
340391

341-
if (this.proxySettingConfiguredManually(ctx) || discoveredProxy)
392+
if (hasManualProxy || discoveredProxy)
342393
{
343-
const finalProxy = ctx?.proxyUrl && ctx?.proxyUrl !== '""' && ctx?.proxyUrl !== '' ? ctx.proxyUrl : discoveredProxy;
344-
ctx.eventStream.post(new ProxyUsed(`Utilizing the Proxy : Manual ? ${ctx?.proxyUrl}, Automatic: ${discoveredProxy}, Decision : ${finalProxy}`))
394+
const finalProxy = manualProxyUrl && manualProxyUrl !== '""' && manualProxyUrl !== '' ? manualProxyUrl : discoveredProxy;
395+
eventStream?.post(new ProxyUsed(`Utilizing the Proxy : Manual ? ${manualProxyUrl}, Automatic: ${discoveredProxy}, Decision : ${finalProxy}`))
345396
const proxyAgent = new HttpsProxyAgent(finalProxy);
346397
return proxyAgent;
347398
}
348399
}
349400
catch (error: any)
350401
{
351-
ctx.eventStream.post(new SuppressedAcquisitionError(error, `The proxy lookup failed, most likely due to limited registry access. Skipping automatic proxy lookup.`));
402+
eventStream?.post(new SuppressedAcquisitionError(error, `The proxy lookup failed, most likely due to limited registry access. Skipping automatic proxy lookup.`));
352403
}
353404

354405
return null;
@@ -485,11 +536,6 @@ If you're on a proxy and disable registry access, you must set the proxy in our
485536
}
486537
}
487538

488-
private proxySettingConfiguredManually(ctx: IAcquisitionWorkerContext): boolean
489-
{
490-
return ctx?.proxyUrl ? ctx?.proxyUrl !== '""' : false;
491-
}
492-
493539
private timeoutMsFromCtx(ctx: IAcquisitionWorkerContext): number
494540
{
495541
return ctx?.timeoutSeconds * 1000;

vscode-dotnet-runtime-library/src/test/unit/WebRequestWorker.test.ts

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,17 @@
55
import * as chai from 'chai';
66

77
import * as chaiAsPromised from 'chai-as-promised';
8+
import * as http from 'http';
9+
import * as https from 'https';
10+
import * as dns from 'dns';
811
import * as path from 'path';
912
import { DotnetCoreAcquisitionWorker } from '../../Acquisition/DotnetCoreAcquisitionWorker';
1013
import { IInstallScriptAcquisitionWorker } from '../../Acquisition/IInstallScriptAcquisitionWorker';
1114
import
1215
{
1316
DotnetFallbackInstallScriptUsed,
1417
DotnetInstallScriptAcquisitionError,
18+
OfflineDetectionLogicTriggered,
1519
WebRequestTime,
1620
} from '../../EventStream/EventStreamEvents';
1721
import
@@ -127,3 +131,148 @@ suite('WebRequestWorker Unit Tests', function ()
127131
});
128132
});
129133

134+
/**
135+
* Helper that intercepts all outbound HTTP/HTTPS requests and DNS lookups to simulate a fully offline machine.
136+
* Blocks at the http/https.request level which is what axios uses internally.
137+
* Returns a restore function that undoes the patching.
138+
*/
139+
function simulateOffline(): () => void
140+
{
141+
const originalHttpsRequest = https.request;
142+
const originalHttpRequest = http.request;
143+
const originalDnsResolve = dns.promises.Resolver.prototype.resolve;
144+
145+
// Block all HTTPS requests
146+
(https as any).request = function (...args: any[])
147+
{
148+
const req = new http.ClientRequest('https://localhost:1');
149+
process.nextTick(() => req.destroy(new Error('simulateOffline: HTTPS request blocked')));
150+
return req;
151+
};
152+
153+
// Block all HTTP requests
154+
(http as any).request = function (...args: any[])
155+
{
156+
const req = new http.ClientRequest('http://localhost:1');
157+
process.nextTick(() => req.destroy(new Error('simulateOffline: HTTP request blocked')));
158+
return req;
159+
};
160+
161+
// Block DNS resolution
162+
dns.promises.Resolver.prototype.resolve = function ()
163+
{
164+
return Promise.reject(new Error('simulateOffline: DNS blocked'));
165+
} as any;
166+
167+
return () =>
168+
{
169+
(https as any).request = originalHttpsRequest;
170+
(http as any).request = originalHttpRequest;
171+
dns.promises.Resolver.prototype.resolve = originalDnsResolve;
172+
};
173+
}
174+
175+
/**
176+
* Helper that blocks only DNS resolution but allows TCP/TLS connections through.
177+
* Simulates a proxy environment where DNS doesn't resolve locally but HTTP works.
178+
*/
179+
function simulateDnsOnlyFailure(): () => void
180+
{
181+
const originalDnsResolve = dns.promises.Resolver.prototype.resolve;
182+
183+
dns.promises.Resolver.prototype.resolve = function ()
184+
{
185+
return Promise.reject(new Error('simulateDnsOnlyFailure: DNS blocked'));
186+
} as any;
187+
188+
return () =>
189+
{
190+
dns.promises.Resolver.prototype.resolve = originalDnsResolve;
191+
};
192+
}
193+
194+
suite('isOnline Connectivity Detection Tests', function ()
195+
{
196+
this.afterEach(async () =>
197+
{
198+
// Reset the singleton so each test gets a fresh instance
199+
(WebRequestWorkerSingleton as any).instance = undefined;
200+
});
201+
202+
test('isOnline returns false when fully offline (DNS + HTTP both blocked)', async () =>
203+
{
204+
const eventStream = new MockEventStream();
205+
206+
// Reset singleton so the new instance is created while network is blocked
207+
(WebRequestWorkerSingleton as any).instance = undefined;
208+
209+
const restoreNetwork = simulateOffline();
210+
try
211+
{
212+
const result = await WebRequestWorkerSingleton.getInstance().isOnline(5, eventStream);
213+
assert.isFalse(result, 'Should report offline when all network is blocked');
214+
215+
const offlineEvent = eventStream.events.find(event => event instanceof OfflineDetectionLogicTriggered);
216+
assert.exists(offlineEvent, 'Should log an offline detection event for the DNS failure');
217+
}
218+
finally
219+
{
220+
restoreNetwork();
221+
}
222+
}).timeout(15000);
223+
224+
test('isOnline returns true when DNS fails but HTTP succeeds (proxy environment)', async () =>
225+
{
226+
const eventStream = new MockEventStream();
227+
const restoreNetwork = simulateDnsOnlyFailure();
228+
try
229+
{
230+
const result = await WebRequestWorkerSingleton.getInstance().isOnline(5, eventStream);
231+
assert.isTrue(result, 'Should report online when DNS fails but HTTP HEAD succeeds');
232+
233+
const dnsFailEvent = eventStream.events.find(event =>
234+
event instanceof OfflineDetectionLogicTriggered &&
235+
event.supplementalMessage.includes('DNS resolution failed'));
236+
assert.exists(dnsFailEvent, 'Should log a DNS failure event');
237+
238+
const httpSuccessEvent = eventStream.events.find(event =>
239+
event instanceof OfflineDetectionLogicTriggered &&
240+
event.supplementalMessage.includes('HTTP connectivity confirmed'));
241+
assert.exists(httpSuccessEvent, 'Should log that HTTP fallback succeeded');
242+
}
243+
finally
244+
{
245+
restoreNetwork();
246+
}
247+
}).timeout(15000);
248+
249+
test('isOnline returns true when DNS succeeds (normal environment)', async () =>
250+
{
251+
const eventStream = new MockEventStream();
252+
const result = await WebRequestWorkerSingleton.getInstance().isOnline(5, eventStream);
253+
assert.isTrue(result, 'Should report online when DNS resolves successfully');
254+
}).timeout(15000);
255+
256+
test('isOnline returns false when DOTNET_INSTALL_TOOL_OFFLINE env var is set', async () =>
257+
{
258+
const eventStream = new MockEventStream();
259+
const originalEnv = process.env.DOTNET_INSTALL_TOOL_OFFLINE;
260+
process.env.DOTNET_INSTALL_TOOL_OFFLINE = '1';
261+
try
262+
{
263+
const result = await WebRequestWorkerSingleton.getInstance().isOnline(5, eventStream);
264+
assert.isFalse(result, 'Should report offline when DOTNET_INSTALL_TOOL_OFFLINE=1');
265+
}
266+
finally
267+
{
268+
if (originalEnv === undefined)
269+
{
270+
delete process.env.DOTNET_INSTALL_TOOL_OFFLINE;
271+
}
272+
else
273+
{
274+
process.env.DOTNET_INSTALL_TOOL_OFFLINE = originalEnv;
275+
}
276+
}
277+
}).timeout(5000);
278+
});

vscode-dotnet-runtime-library/yarn.lock

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

0 commit comments

Comments
 (0)