diff --git a/vscode-dotnet-runtime-extension/CHANGELOG.md b/vscode-dotnet-runtime-extension/CHANGELOG.md index 7bebe21e72..ac83dd556f 100644 --- a/vscode-dotnet-runtime-extension/CHANGELOG.md +++ b/vscode-dotnet-runtime-extension/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning]. ## [Unreleased] +### Fixed +- Improved offline detection so .NET installs no longer fail with a misleading "may be offline" error on machines where the c-ares DNS resolver fails but the system is online (e.g. VPN / proxy / hosts-file setups). Hung network requests now also fall back to the native fetch client quickly instead of waiting for the full install timeout. ## [3.1.0] - 2026-5 diff --git a/vscode-dotnet-runtime-library/src/Utils/WebRequestWorkerSingleton.ts b/vscode-dotnet-runtime-library/src/Utils/WebRequestWorkerSingleton.ts index 41a6abef67..2141e9fe0b 100644 --- a/vscode-dotnet-runtime-library/src/Utils/WebRequestWorkerSingleton.ts +++ b/vscode-dotnet-runtime-library/src/Utils/WebRequestWorkerSingleton.ts @@ -153,22 +153,24 @@ export class WebRequestWorkerSingleton throw new EventBasedError('AxiosGetFailedWithInvalidURL', `Request to the url ${url} failed, as the URL is invalid.`); } const timeoutCancelTokenHook = new AbortController(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const attemptTimeoutMs = this.attemptTimeoutMs(ctx, (options as any)?.responseType === 'stream'); // eslint-disable-next-line @typescript-eslint/no-misused-promises const timeout = setTimeout(async () => { timeoutCancelTokenHook.abort(); - ctx.eventStream.post(new WebRequestTime(`Timer for request:`, String(this.timeoutMsFromCtx(ctx)), 'false', url, '777')); // 777 for custom abort status. arbitrary + ctx.eventStream.post(new WebRequestTime(`Timer for request:`, String(attemptTimeoutMs), 'false', url, '777')); // 777 for custom abort status. arbitrary if (!(await this.isOnline(ctx.timeoutSeconds, ctx.eventStream))) { const offlineError = new EventBasedError('DotnetOfflineFailure', 'No internet connection detected: Cannot install .NET'); ctx.eventStream.post(new DotnetOfflineFailure(offlineError, null)); throw offlineError; } - const formattedError = new Error(`TIMEOUT: The request to ${url} timed out at ${ctx.timeoutSeconds} s. This only occurs if your internet + const formattedError = new Error(`TIMEOUT: The request to ${url} timed out at ${attemptTimeoutMs} ms. This only occurs if your internet or the url are experiencing connection difficulties; not if the server is being slow to respond. Check your connection, the url, and or increase the timeout value here: https://github.com/dotnet/vscode-dotnet-runtime/blob/main/Documentation/troubleshooting-runtime.md#install-script-timeouts`); ctx.eventStream.post(new WebRequestError(new EventBasedError('WebRequestError', formattedError.message, formattedError.stack), null)); throw formattedError; - }, this.timeoutMsFromCtx(ctx)); + }, attemptTimeoutMs); // Make the web request let response; @@ -194,7 +196,7 @@ export class WebRequestWorkerSingleton ctx.eventStream.post(new WebRequestUsingAltClient(url, `Using fetch over axios, as axios failed. Axios failure: ${this.clientCreationError ? JSON.stringify(this.clientCreationError) : ''}`)); try { - const response = await fetch(url, { signal: AbortSignal.timeout(ctx.timeoutSeconds * 1000) }); + const response = await fetch(url, { signal: AbortSignal.timeout(this.attemptTimeoutMs(ctx, returnDownloadStream)) }); if (url.includes('json')) { const responseJson = await response.json(); @@ -220,11 +222,12 @@ export class WebRequestWorkerSingleton /** * @returns The data from a web request that was hopefully cached. Even if it wasn't cached, we will make an attempt to get the data. + * @param requestOptions Optional per-request axios options (e.g. cache settings) merged into the request. * @remarks This function is no longer needed as the data is cached either way if you call makeWebRequest, but it was kept to prevent breaking APIs. */ - public async getCachedData(url: string, ctx: IAcquisitionWorkerContext, retriesCount = 2): Promise + public async getCachedData(url: string, ctx: IAcquisitionWorkerContext, retriesCount = 2, requestOptions?: object): Promise { - return this.makeWebRequest(url, ctx, true, retriesCount); + return this.makeWebRequest(url, ctx, true, retriesCount, requestOptions); } private reportTimeAnalytics(response: any, options: any, url: string, ctx: IAcquisitionWorkerContext, manualFinalTime: bigint | null = null): void @@ -269,6 +272,34 @@ export class WebRequestWorkerSingleton } } + /** + * Resolve a hostname to a single address using the system resolver (dns.lookup). + * dns.lookup uses the same resolution path as real socket connections (hosts file, + * VPN adapters, system DNS), unlike a c-ares Resolver which reads a static DNS server + * list and can report a false "offline" (e.g. ECONNREFUSED on Windows) even when the + * machine has full connectivity. + * @returns the resolved address, or undefined if it could not be resolved within timeoutMs. + * @remarks Protected so tests can substitute a fake resolver. + */ + protected async resolveHostnameForOnlineCheck(hostName: string, timeoutMs: number): Promise + { + let timer: NodeJS.Timeout | undefined; + try + { + return await Promise.race([ + dns.promises.lookup(hostName).then(({ address }) => address), + new Promise(resolve => { timer = setTimeout(() => resolve(undefined), timeoutMs); }) + ]); + } + finally + { + if (timer) + { + clearTimeout(timer); + } + } + } + public async isOnline(timeoutSec: number, eventStream: IEventStream): Promise { if (process.env.DOTNET_INSTALL_TOOL_OFFLINE === '1') @@ -276,21 +307,27 @@ export class WebRequestWorkerSingleton return false; } - const microsoftServerHostName = 'www.microsoft.com'; - const expectedDNSResolutionTimeMs = Math.max(timeoutSec * 2, 100); // Assumption: DNS resolution should take less than 1/50th of the time it'd take to download .NET. - // ... 100 ms is there as a default to prevent the dns resolver from throwing a runtime error if the user sets timeoutSeconds to 0. - - const dnsResolver = new dns.promises.Resolver({ timeout: expectedDNSResolutionTimeMs }); - const couldConnect = await dnsResolver.resolve(microsoftServerHostName).then(() => + let address: string | undefined; + try { - return true; - }).catch((error: any) => + // ~1/50th of the download budget, with a 5s floor. Previously the budget was timeoutSec*2 + // (1.2s at the default 600s) — far below the 1/50th the original comment described — and used + // a c-ares Resolver that can misdetect "offline" on networks where dns.lookup works fine. + address = await this.resolveHostnameForOnlineCheck('www.microsoft.com', Math.max(timeoutSec * 20, 5000)); + } + catch (error: any) { eventStream.post(new OfflineDetectionLogicTriggered((error as EventCancellationError), `DNS resolution failed at microsoft.com, ${JSON.stringify(error)}.`)); return false; - }); + } + + if (!address) + { + eventStream.post(new OfflineDetectionLogicTriggered(new EventCancellationError('OfflineDetectionLogicTriggered', 'DNS resolution timed out at microsoft.com.'), `DNS resolution timed out at microsoft.com.`)); + return false; + } - return couldConnect; + return true; } /** * @@ -416,9 +453,9 @@ export class WebRequestWorkerSingleton private async getAxiosOptions(ctx: IAcquisitionWorkerContext, numRetries: number, furtherOptions?: object, keepAlive = true): Promise { const proxyAgent = await this.GetProxyAgentIfNeeded(ctx); - const options: object = { - timeout: this.timeoutMsFromCtx(ctx), + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + timeout: this.attemptTimeoutMs(ctx, ((furtherOptions as any)?.responseType === 'stream')), 'axios-retry': { retries: numRetries }, ...(keepAlive && { headers: { 'Connection': 'keep-alive' } }), ...(proxyAgent !== null && { proxy: false }), @@ -433,13 +470,14 @@ export class WebRequestWorkerSingleton * * @param throwOnError Should we throw if the connection fails, there's a bad URL passed in, or something else goes wrong? * @param numRetries The number of retry attempts if the url is not giving a good response. + * @param requestOptions Optional per-request axios options (e.g. cache settings) merged into the request. * @returns The data returned from a get request to the url. It may be of string type, but it may also be of another type if the return result is convert-able (e.g. JSON.) * @remarks protected for ease of testing. */ - protected async makeWebRequest(url: string, ctx: IAcquisitionWorkerContext, throwOnError: boolean, numRetries: number): Promise + protected async makeWebRequest(url: string, ctx: IAcquisitionWorkerContext, throwOnError: boolean, numRetries: number, requestOptions?: object): Promise { ctx.eventStream.post(new WebRequestInitiated(`Making Web Request For ${url}`)); - const options = await this.getAxiosOptions(ctx, numRetries); + const options = { ...await this.getAxiosOptions(ctx, numRetries), ...requestOptions }; try { @@ -494,4 +532,24 @@ If you're on a proxy and disable registry access, you must set the proxy in our { return ctx?.timeoutSeconds * 1000; } + + /** + * The timeout to use for a single request attempt. Downloads (streamed responses) keep the + * full user-configured budget so large installers aren't cut off on slow connections, while + * non-download requests use a bounded per-attempt timeout so a hung connection (e.g. one + * routed through a broken proxy) fails quickly and falls back to the native fetch client. + */ + private attemptTimeoutMs(ctx: IAcquisitionWorkerContext, isStreaming: boolean): number + { + if (isStreaming) + { + return this.timeoutMsFromCtx(ctx); + } + // 30_000 ms (30s) is the max time to wait on a single non-download request attempt before + // giving up on that attempt (it is then retried via the native fetch fallback). This prevents + // a hung connection — e.g. one routed through a broken/partial proxy — from blocking for the + // full user-configured timeout (default 600s). Streaming downloads keep the full budget so + // large installers aren't cut off. + return Math.min(this.timeoutMsFromCtx(ctx), 30_000); + } } diff --git a/vscode-dotnet-runtime-library/src/test/mocks/MockObjects.ts b/vscode-dotnet-runtime-library/src/test/mocks/MockObjects.ts index 0731eea281..377f5c5d1d 100644 --- a/vscode-dotnet-runtime-library/src/test/mocks/MockObjects.ts +++ b/vscode-dotnet-runtime-library/src/test/mocks/MockObjects.ts @@ -247,13 +247,13 @@ export class MockTrackingWebRequestWorker extends WebRequestWorkerSingleton { this.requestCount++; } - protected async makeWebRequest(url: string, ctx: IAcquisitionWorkerContext, shouldThrow = false, retries = 2): Promise + protected async makeWebRequest(url: string, ctx: IAcquisitionWorkerContext, shouldThrow = false, retries = 2, requestOptions?: object): Promise { if (!(await this.isUrlCached(url, ctx))) { this.incrementRequestCount(); } - return super.makeWebRequest(url, ctx, shouldThrow, retries); + return super.makeWebRequest(url, ctx, shouldThrow, retries, requestOptions); } } diff --git a/vscode-dotnet-runtime-library/src/test/unit/WebRequestWorker.test.ts b/vscode-dotnet-runtime-library/src/test/unit/WebRequestWorker.test.ts index 490f606941..1b870acfb4 100644 --- a/vscode-dotnet-runtime-library/src/test/unit/WebRequestWorker.test.ts +++ b/vscode-dotnet-runtime-library/src/test/unit/WebRequestWorker.test.ts @@ -12,6 +12,7 @@ import { DotnetFallbackInstallScriptUsed, DotnetInstallScriptAcquisitionError, + OfflineDetectionLogicTriggered, WebRequestTime, } from '../../EventStream/EventStreamEvents'; import @@ -35,6 +36,29 @@ const maxTimeoutTime = 10000; // Website used for the sake of it returning the same response always (tm) const staticWebsiteUrl = 'https://builds.dotnet.microsoft.com/dotnet/release-metadata/2.1/releases.json'; +// A WebRequestWorkerSingleton whose hostname resolution for isOnline() is fully controlled by the test, +// so we don't depend on real DNS (which is exactly what the c-ares resolver incorrectly fails on). +class MockOnlineWebRequestWorker extends WebRequestWorkerSingleton +{ + public resolvedAddress: string | undefined = '1.2.3.4'; + public shouldThrow = false; + + constructor() + { + super(); + const _ = WebRequestWorkerSingleton.getInstance(); // cause super to exist + } + + protected async resolveHostnameForOnlineCheck(hostName: string, timeoutMs: number): Promise + { + if (this.shouldThrow) + { + throw new Error('ENOTFOUND test'); + } + return this.resolvedAddress; + } +} + suite('WebRequestWorker Unit Tests', function () { this.afterEach(async () => @@ -114,13 +138,16 @@ suite('WebRequestWorker Unit Tests', function () const uri = 'https://microsoft.com'; const webWorker = new MockTrackingWebRequestWorker(true); - const uncachedResult = await webWorker.getCachedData(uri, ctx); - await new Promise(resolve => setTimeout(resolve, 120000)); - const cachedResult = await webWorker.getCachedData(uri, ctx); + // Force a short, deterministic cache TTL and stop interpreting the server's cache headers + // (which set ~10 minute max-age and would make the entry outlive the test's 120s wait). + const cacheOptions = { cache: { ttl: 2000, interpretHeader: false } }; + const uncachedResult = await webWorker.getCachedData(uri, ctx, 2, cacheOptions); + await new Promise(resolve => setTimeout(resolve, 3000)); + const cachedResult = await webWorker.getCachedData(uri, ctx, 2, cacheOptions); assert.exists(uncachedResult); const requestCount = webWorker.getRequestCount(); assert.isAtLeast(requestCount, 2); - }).timeout((maxTimeoutTime * 7) + 120000); + }).timeout(maxTimeoutTime * 7); test('It actually times requests', async () => { @@ -134,6 +161,49 @@ suite('WebRequestWorker Unit Tests', function () assert.equal(timerEvents?.finished, 'true', 'The timed event time finished'); assert.isTrue(Number(timerEvents?.durationMs) > 0, 'The timed event time is > 0'); assert.isTrue(String(timerEvents?.status).startsWith('2'), 'The timed event has a status 2XX'); + }).timeout(maxTimeoutTime); + + test('isOnline returns true when the hostname resolves', async () => + { + const eventStream = new MockEventStream(); + const worker = new MockOnlineWebRequestWorker(); + worker.resolvedAddress = '1.2.3.4'; + assert.isTrue(await worker.isOnline(600, eventStream)); + assert.isEmpty(eventStream.events.filter(event => event instanceof OfflineDetectionLogicTriggered)); + }); + + test('isOnline returns false and reports an event when the hostname does not resolve', async () => + { + const eventStream = new MockEventStream(); + const worker = new MockOnlineWebRequestWorker(); + worker.resolvedAddress = undefined; + assert.isFalse(await worker.isOnline(600, eventStream)); + assert.exists(eventStream.events.find(event => event instanceof OfflineDetectionLogicTriggered)); + }); + + test('isOnline reports an event when DNS lookup errors', async () => + { + const eventStream = new MockEventStream(); + const worker = new MockOnlineWebRequestWorker(); + worker.shouldThrow = true; + assert.isFalse(await worker.isOnline(600, eventStream)); + assert.exists(eventStream.events.find(event => event instanceof OfflineDetectionLogicTriggered)); + }); + + test('isOnline returns false when DOTNET_INSTALL_TOOL_OFFLINE is set', async () => + { + const eventStream = new MockEventStream(); + const worker = new MockOnlineWebRequestWorker(); + worker.resolvedAddress = '1.2.3.4'; + process.env.DOTNET_INSTALL_TOOL_OFFLINE = '1'; + try + { + assert.isFalse(await worker.isOnline(600, eventStream)); + } + finally + { + delete process.env.DOTNET_INSTALL_TOOL_OFFLINE; + } }); }); diff --git a/vscode-dotnet-sdk-extension/CHANGELOG.md b/vscode-dotnet-sdk-extension/CHANGELOG.md index 822586392a..4aa1ffb2cc 100644 --- a/vscode-dotnet-sdk-extension/CHANGELOG.md +++ b/vscode-dotnet-sdk-extension/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning]. ## [Unreleased] +### Fixed + +- Improved offline detection (shared library) so SDK installs no longer fail with a misleading "may be offline" error when DNS resolution fails but the machine is online. + ### Added - A new error type for when the user tries to install via the Powershell install script on a system where powershell cannot be found [#1212]