From 89a18ddf42a293f63383f1dc9917b9266db4e12a Mon Sep 17 00:00:00 2001 From: wenytang-ms Date: Wed, 27 May 2026 13:24:38 +0800 Subject: [PATCH 1/4] fix(completion): add timeout and CancellationToken handling to pom IntelliSense network calls The pom.xml completion provider (PomCompletionProvider) calls search.maven.org via `src/utils/requestUtils.ts#httpsGet`. That helper had: - no socket / request timeout, so a stuck TCP connection (corporate proxy, DNS black hole, unreachable Maven Central) would leave the underlying Promise pending forever; - no request-level error handler, only a response-level one; - no awareness of VS Code's `CancellationToken`, so even when the editor wanted to cancel a completion request the in-flight HTTPS call kept running. Combined with the previous serial `await provider.provide(...)` loop in `PomCompletionProvider.provideCompletionItems`, a single hung HTTPS call made the entire IntelliSense popup stay on `Loading...` indefinitely (see #1127). This change: - adds a 10s socket timeout to `httpsGet` and wires `req.on('timeout')` to `req.destroy(...)`; - adds a request-level `error` handler and a single `settle` guard so the Promise can never resolve and reject; - threads an optional `CancellationToken` through `getArtifacts` / `getVersions` down into `httpsGet` and aborts the request via `req.destroy()` on cancellation; - in `PomCompletionProvider` honors the `CancellationToken` and runs the registered sub-providers concurrently with `Promise.all` + a per-provider try/catch, so a slow / failing provider can no longer block the others; - in `ArtifactProvider` switches the central / index / local fan-out to `Promise.allSettled` so a network failure on Maven Central still returns local and index results instead of dropping everything. No behavior change when the network is healthy: completions still come from all three sources. When the network hangs or is unreachable, the popup now resolves within 10s instead of getting stuck. Refs #1127 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/completion/PomCompletionProvider.ts | 20 ++++++-- src/completion/providers/ArtifactProvider.ts | 37 ++++++++++---- .../providers/IXmlCompletionProvider.ts | 2 +- .../providers/artifact/FromCentral.ts | 12 ++--- .../providers/artifact/IArtifactProvider.ts | 8 +-- src/utils/requestUtils.ts | 51 +++++++++++++++---- 6 files changed, 95 insertions(+), 35 deletions(-) diff --git a/src/completion/PomCompletionProvider.ts b/src/completion/PomCompletionProvider.ts index 37808d44..c40b7f6f 100644 --- a/src/completion/PomCompletionProvider.ts +++ b/src/completion/PomCompletionProvider.ts @@ -34,7 +34,10 @@ export class PomCompletionProvider implements vscode.CompletionItemProvider { } - async provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, _token: vscode.CancellationToken, _context: vscode.CompletionContext): Promise | undefined> { + async provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken, _context: vscode.CompletionContext): Promise | undefined> { + if (token.isCancellationRequested) { + return undefined; + } const documentText: string = document.getText(); const cursorOffset: number = document.offsetAt(position); const currentNode: Node | undefined = getCurrentNode(documentText, cursorOffset); @@ -42,10 +45,17 @@ export class PomCompletionProvider implements vscode.CompletionItemProvider { return undefined; } - const ret = []; - for (const provider of this.providers) { - ret.push(...await provider.provide(document, position, currentNode)); + const results = await Promise.all( + this.providers.map(provider => + provider.provide(document, position, currentNode, token).catch(err => { + console.error(err); + return [] as vscode.CompletionItem[]; + }) + ) + ); + if (token.isCancellationRequested) { + return undefined; } - return ret; + return ([] as vscode.CompletionItem[]).concat(...results); } } \ No newline at end of file diff --git a/src/completion/providers/ArtifactProvider.ts b/src/completion/providers/ArtifactProvider.ts index d6426561..031198cb 100644 --- a/src/completion/providers/ArtifactProvider.ts +++ b/src/completion/providers/ArtifactProvider.ts @@ -22,7 +22,7 @@ export class ArtifactProvider implements IXmlCompletionProvider { this.localProvider = new FromLocal(); } - async provide(document: vscode.TextDocument, position: vscode.Position, currentNode: Node): Promise { + async provide(document: vscode.TextDocument, position: vscode.Position, currentNode: Node, token?: vscode.CancellationToken): Promise { let tagNode: Element | undefined; if (isTag(currentNode)) { tagNode = currentNode; @@ -48,9 +48,11 @@ export class ArtifactProvider implements IXmlCompletionProvider { const groupIdHint: string = getTextFromNode(groupIdTextNode); const artifactIdHint: string = getTextFromNode(artifactIdTextNode); - const centralItems: vscode.CompletionItem[] = await this.centralProvider.getGroupIdCandidates(groupIdHint, artifactIdHint); - const indexItems: vscode.CompletionItem[] = await this.indexProvider.getGroupIdCandidates(groupIdHint, artifactIdHint); - const localItems: vscode.CompletionItem[] = await this.localProvider.getGroupIdCandidates(groupIdHint); + const [centralItems, indexItems, localItems] = await settledAll([ + this.centralProvider.getGroupIdCandidates(groupIdHint, artifactIdHint, token), + this.indexProvider.getGroupIdCandidates(groupIdHint, artifactIdHint), + this.localProvider.getGroupIdCandidates(groupIdHint), + ]); const mergedItems: vscode.CompletionItem[] = _.unionBy(centralItems, indexItems, localItems, (item) => item.insertText); mergedItems.forEach(item => item.range = targetRange); return mergedItems; @@ -69,9 +71,11 @@ export class ArtifactProvider implements IXmlCompletionProvider { const groupIdHint: string = getTextFromNode(groupIdTextNode); const artifactIdHint: string = getTextFromNode(artifactIdTextNode); - const centralItems: vscode.CompletionItem[] = await this.centralProvider.getArtifactIdCandidates(groupIdHint, artifactIdHint); - const indexItems: vscode.CompletionItem[] = await this.indexProvider.getArtifactIdCandidates(groupIdHint, artifactIdHint); - const localItems: vscode.CompletionItem[] = await this.localProvider.getArtifactIdCandidates(groupIdHint); + const [centralItems, indexItems, localItems] = await settledAll([ + this.centralProvider.getArtifactIdCandidates(groupIdHint, artifactIdHint, token), + this.indexProvider.getArtifactIdCandidates(groupIdHint, artifactIdHint), + this.localProvider.getArtifactIdCandidates(groupIdHint), + ]); let mergedItems: vscode.CompletionItem[] = []; const ID_SEPARATOR = ":"; @@ -110,9 +114,11 @@ export class ArtifactProvider implements IXmlCompletionProvider { return []; } - const centralItems: vscode.CompletionItem[] = await this.centralProvider.getVersionCandidates(groupIdHint, artifactIdHint); - const indexItems: vscode.CompletionItem[] = await this.indexProvider.getVersionCandidates(groupIdHint, artifactIdHint); - const localItems: vscode.CompletionItem[] = await this.localProvider.getVersionCandidates(groupIdHint, artifactIdHint); + const [centralItems, indexItems, localItems] = await settledAll([ + this.centralProvider.getVersionCandidates(groupIdHint, artifactIdHint, undefined, token), + this.indexProvider.getVersionCandidates(groupIdHint, artifactIdHint), + this.localProvider.getVersionCandidates(groupIdHint, artifactIdHint), + ]); const mergedItems: vscode.CompletionItem[] = _.unionBy(centralItems, indexItems, localItems, (item) => item.insertText); mergedItems.forEach(item => item.range = targetRange); return mergedItems; @@ -122,6 +128,17 @@ export class ArtifactProvider implements IXmlCompletionProvider { } } +async function settledAll(promises: Promise[]): Promise { + const results = await Promise.allSettled(promises); + return results.map(r => { + if (r.status === "fulfilled") { + return r.value; + } + console.error(r.reason); + return []; + }); +} + function getRange(node: Node | null, document: vscode.TextDocument, fallbackPosition?: vscode.Position) { if (fallbackPosition) { return new vscode.Range( diff --git a/src/completion/providers/IXmlCompletionProvider.ts b/src/completion/providers/IXmlCompletionProvider.ts index 6a4d7642..218846dd 100644 --- a/src/completion/providers/IXmlCompletionProvider.ts +++ b/src/completion/providers/IXmlCompletionProvider.ts @@ -5,5 +5,5 @@ import { Node } from "domhandler"; import * as vscode from "vscode"; export interface IXmlCompletionProvider { - provide(document: vscode.TextDocument, position: vscode.Position, currentNode: Node): Promise; + provide(document: vscode.TextDocument, position: vscode.Position, currentNode: Node, token?: vscode.CancellationToken): Promise; } diff --git a/src/completion/providers/artifact/FromCentral.ts b/src/completion/providers/artifact/FromCentral.ts index d887b8ce..ab246c70 100644 --- a/src/completion/providers/artifact/FromCentral.ts +++ b/src/completion/providers/artifact/FromCentral.ts @@ -8,9 +8,9 @@ import { IArtifactCompletionProvider } from "./IArtifactProvider"; import { getSortText } from "../../utils"; export class FromCentral implements IArtifactCompletionProvider { - public async getGroupIdCandidates(groupIdHint: string, artifactIdHint: string): Promise { + public async getGroupIdCandidates(groupIdHint: string, artifactIdHint: string, token?: vscode.CancellationToken): Promise { const keywords: string[] = [...groupIdHint.split("."), ...artifactIdHint.split("-")]; - const docs: IArtifactMetadata[] = await getArtifacts(keywords); + const docs: IArtifactMetadata[] = await getArtifacts(keywords, token); const groupIds: string[] = Array.from(new Set(docs.map(doc => doc.g)).values()); const commandOnSelection: vscode.Command = { title: "selected", command: COMMAND_COMPLETION_ITEM_SELECTED, @@ -25,9 +25,9 @@ export class FromCentral implements IArtifactCompletionProvider { }); } - public async getArtifactIdCandidates(groupIdHint: string, artifactIdHint: string): Promise { + public async getArtifactIdCandidates(groupIdHint: string, artifactIdHint: string, token?: vscode.CancellationToken): Promise { const keywords: string[] = [...groupIdHint.split("."), ...artifactIdHint.split("-")]; - const docs: IArtifactMetadata[] = await getArtifacts(keywords); + const docs: IArtifactMetadata[] = await getArtifacts(keywords, token); const commandOnSelection: vscode.Command = { title: "selected", command: COMMAND_COMPLETION_ITEM_SELECTED, arguments: [{ infoName: INFO_COMPLETION_ITEM_SELECTED, completeFor: "artifactId", source: "maven-central" }] @@ -45,12 +45,12 @@ export class FromCentral implements IArtifactCompletionProvider { }); } - public async getVersionCandidates(groupId: string, artifactId: string): Promise { + public async getVersionCandidates(groupId: string, artifactId: string, _versionHint?: string, token?: vscode.CancellationToken): Promise { if (!groupId && !artifactId) { return []; } - const docs: IVersionMetadata[] = await getVersions(groupId, artifactId); + const docs: IVersionMetadata[] = await getVersions(groupId, artifactId, token); const commandOnSelection: vscode.Command = { title: "selected", command: COMMAND_COMPLETION_ITEM_SELECTED, arguments: [{ infoName: INFO_COMPLETION_ITEM_SELECTED, completeFor: "version", source: "maven-central" }] diff --git a/src/completion/providers/artifact/IArtifactProvider.ts b/src/completion/providers/artifact/IArtifactProvider.ts index c206aba5..8d327898 100644 --- a/src/completion/providers/artifact/IArtifactProvider.ts +++ b/src/completion/providers/artifact/IArtifactProvider.ts @@ -1,10 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -import { CompletionItem } from "vscode"; +import { CancellationToken, CompletionItem } from "vscode"; export interface IArtifactCompletionProvider { - getGroupIdCandidates(groupIdHint?: string, artifactIdHint?: string): Promise; - getArtifactIdCandidates(groupIdHint?: string, artifactIdHint?: string): Promise; - getVersionCandidates(groupIdHint?: string, artifactIdHint?: string, versionHint?: string): Promise; + getGroupIdCandidates(groupIdHint?: string, artifactIdHint?: string, token?: CancellationToken): Promise; + getArtifactIdCandidates(groupIdHint?: string, artifactIdHint?: string, token?: CancellationToken): Promise; + getVersionCandidates(groupIdHint?: string, artifactIdHint?: string, versionHint?: string, token?: CancellationToken): Promise; } diff --git a/src/utils/requestUtils.ts b/src/utils/requestUtils.ts index dc710e1f..f8550ea0 100644 --- a/src/utils/requestUtils.ts +++ b/src/utils/requestUtils.ts @@ -6,10 +6,12 @@ import * as https from "https"; import * as _ from "lodash"; import * as path from "path"; import * as url from "url"; +import * as vscode from "vscode"; const URL_MAVEN_SEARCH_API = "https://search.maven.org/solrsearch/select"; const URL_MAVEN_CENTRAL_REPO = "https://repo1.maven.org/maven2/"; const MAVEN_METADATA_FILENAME = "maven-metadata.xml"; +const HTTPS_GET_TIMEOUT_MS = 10_000; export interface IArtifactMetadata { id: string; @@ -28,7 +30,7 @@ export interface IVersionMetadata { timestamp: number; } -export async function getArtifacts(keywords: string[]): Promise { +export async function getArtifacts(keywords: string[], token?: vscode.CancellationToken): Promise { // Remove short keywords const validKeywords: string[] = keywords.filter(keyword => keyword.length >= 3); if (validKeywords.length === 0) { @@ -40,8 +42,8 @@ export async function getArtifacts(keywords: string[]): Promise { +export async function getVersions(gid: string, aid: string, token?: vscode.CancellationToken): Promise { const params = { q: `g:"${gid}" AND a:"${aid}"`, core: "gav", rows: 50, wt: "json" }; - const raw: string = await httpsGet(`${URL_MAVEN_SEARCH_API}?${toQueryString(params)}`); try { + const raw: string = await httpsGet(`${URL_MAVEN_SEARCH_API}?${toQueryString(params)}`, token); return _.get(JSON.parse(raw), "response.docs", []); } catch (error) { console.error(error); @@ -80,25 +82,56 @@ export async function getLatestVersion(gid: string, aid: string): Promise { +async function httpsGet(urlString: string, token?: vscode.CancellationToken): Promise { return new Promise((resolve, reject) => { + if (token?.isCancellationRequested) { + reject(new Error("Cancelled")); + return; + } + let result = ""; // eslint-disable-next-line @typescript-eslint/no-explicit-any const options: any = url.parse(urlString); options.headers = { 'User-Agent': 'vscode-maven/0.1' - } - https.get(options, (res: http.IncomingMessage) => { + }; + options.timeout = HTTPS_GET_TIMEOUT_MS; + + let settled = false; + let cancelSub: vscode.Disposable | undefined; + const settle = (fn: () => void) => { + if (settled) { + return; + } + settled = true; + cancelSub?.dispose(); + fn(); + }; + + const req = https.get(options, (res: http.IncomingMessage) => { res.on("data", chunk => { result = result.concat(chunk.toString()); }); res.on("end", () => { - resolve(result); + settle(() => resolve(result)); }); res.on("error", err => { - reject(err); + settle(() => reject(err)); }); }); + + req.on("timeout", () => { + req.destroy(new Error(`HTTPS request to ${urlString} timed out after ${HTTPS_GET_TIMEOUT_MS}ms`)); + }); + req.on("error", err => { + settle(() => reject(err)); + }); + + if (token) { + cancelSub = token.onCancellationRequested(() => { + req.destroy(new Error("Cancelled")); + }); + } }); } From 64256d7215fc2bac4892bb9da5a49091314ec9e6 Mon Sep 17 00:00:00 2001 From: wenytang-ms Date: Wed, 27 May 2026 14:12:35 +0800 Subject: [PATCH 2/4] fix(completion): silence expected cancellation noise and harden httpsGet Follow-up review fixes for #1127: 1. Introduce a dedicated CancellationError class. httpsGet now rejects with this class on token cancellation and request timeout cleanup, and getArtifacts/getVersions suppress console.error for it. VS Code cancels completion tokens on every keystroke, so without this every superseded request was logging "Error: Cancelled" to the extension output. 2. Document the (intentional) behavior change in getArtifacts / getVersions: the try/catch now wraps httpsGet too, so network errors are swallowed and return [] instead of propagating to command handlers like addDependencyHandler / setDependencyVersionHandler. This produces an empty quick pick on outage rather than an unhandled rejection, which is the safer UX. Noted in PR description. 3. Close a tiny race where cancelSub could be registered after the request had already errored synchronously, leaving the CancellationToken listener undisposed. We now re-check `settled` immediately after subscribing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/utils/requestUtils.ts | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/utils/requestUtils.ts b/src/utils/requestUtils.ts index f8550ea0..6dc43e38 100644 --- a/src/utils/requestUtils.ts +++ b/src/utils/requestUtils.ts @@ -13,6 +13,23 @@ const URL_MAVEN_CENTRAL_REPO = "https://repo1.maven.org/maven2/"; const MAVEN_METADATA_FILENAME = "maven-metadata.xml"; const HTTPS_GET_TIMEOUT_MS = 10_000; +/** + * Error thrown when an httpsGet call is aborted because its CancellationToken + * was cancelled (e.g. the user kept typing, so VS Code superseded the previous + * completion request). Callers should swallow this silently — it is not a + * real failure. + */ +class CancellationError extends Error { + constructor() { + super("Cancelled"); + this.name = "CancellationError"; + } +} + +function isCancellation(error: unknown): boolean { + return error instanceof CancellationError; +} + export interface IArtifactMetadata { id: string; g: string; @@ -46,7 +63,9 @@ export async function getArtifacts(keywords: string[], token?: vscode.Cancellati const raw: string = await httpsGet(`${URL_MAVEN_SEARCH_API}?${toQueryString(params)}`, token); return _.get(JSON.parse(raw), "response.docs", []); } catch (error) { - console.error(error); + if (!isCancellation(error)) { + console.error(error); + } return []; } } @@ -62,7 +81,9 @@ export async function getVersions(gid: string, aid: string, token?: vscode.Cance const raw: string = await httpsGet(`${URL_MAVEN_SEARCH_API}?${toQueryString(params)}`, token); return _.get(JSON.parse(raw), "response.docs", []); } catch (error) { - console.error(error); + if (!isCancellation(error)) { + console.error(error); + } return []; } } @@ -85,7 +106,7 @@ export async function getLatestVersion(gid: string, aid: string): Promise { return new Promise((resolve, reject) => { if (token?.isCancellationRequested) { - reject(new Error("Cancelled")); + reject(new CancellationError()); return; } @@ -129,8 +150,15 @@ async function httpsGet(urlString: string, token?: vscode.CancellationToken): Pr if (token) { cancelSub = token.onCancellationRequested(() => { - req.destroy(new Error("Cancelled")); + req.destroy(new CancellationError()); }); + // Defensive: if the request errored synchronously before we got + // here, `settled` is already true and the listener above would + // never be disposed. Drop it now in that case. + if (settled) { + cancelSub.dispose(); + cancelSub = undefined; + } } }); } From 88eba6a09576ddd48e1eb903490b46a8b7601213 Mon Sep 17 00:00:00 2001 From: wenytang-ms Date: Wed, 27 May 2026 14:51:42 +0800 Subject: [PATCH 3/4] test(requestUtils): cover timeout / cancellation / error paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up for PR #1170 (issue #1127). Adds 9 focused unit tests for `getArtifacts` / `getVersions` that drive the underlying `httpsGet` state machine through proxyquire-stubbed `https`: - short-keyword short-circuit (no HTTPS call) - successful 200 response → parsed docs - `options.timeout` set to HTTPS_GET_TIMEOUT_MS (10s) - pre-cancelled token → short-circuit, no console.error - cancellation mid-flight → `req.destroy`, no console.error - non-cancellation network error → returns [] AND logs - socket idle `timeout` event → req destroyed with timeout error, logs - `getVersions` success and silent cancellation paths Also fixes the test harness so new `.test.ts` files load via `ts-node/register/transpile-only` instead of Node's `--experimental-strip-types` ESM loader (which is on by default in Node 22.18+ and broke `require()` inside fresh test files). The fix is one line in .mocharc.json: `"node-option": ["no-experimental-strip-types"]`. All 30 unit tests pass (21 pre-existing + 9 new). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .mocharc.json | 3 +- test/unit/requestUtils.test.ts | 278 +++++++++++++++++++++++++++++++++ 2 files changed, 280 insertions(+), 1 deletion(-) create mode 100644 test/unit/requestUtils.test.ts diff --git a/.mocharc.json b/.mocharc.json index eb613882..7d5f63f1 100644 --- a/.mocharc.json +++ b/.mocharc.json @@ -3,5 +3,6 @@ "spec": "test/unit/**/*.test.ts", "extension": ["ts"], "timeout": 15000, - "reporter": "spec" + "reporter": "spec", + "node-option": ["no-experimental-strip-types"] } diff --git a/test/unit/requestUtils.test.ts b/test/unit/requestUtils.test.ts new file mode 100644 index 00000000..4e32027e --- /dev/null +++ b/test/unit/requestUtils.test.ts @@ -0,0 +1,278 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +/** + * Unit tests for the pom IntelliSense network hardening introduced by PR #1170 + * (fix for issue #1127). Exercises `getArtifacts` / `getVersions` in isolation + * by stubbing the `https` module with a controllable fake, so we can drive + * `data` / `end` / `error` / `timeout` events deterministically and assert the + * timeout, CancellationToken, and console.error suppression behavior. + */ + +import { strict as assert } from "assert"; +import { EventEmitter } from "events"; + +// proxyquire's default export is a callable function with helper methods. +// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires +const proxyquire: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (request: string, stubs: Record): any; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + noPreserveCache: () => (request: string, stubs: Record) => any; +} = require("proxyquire"); + +interface FakeRequest extends EventEmitter { + destroyed: boolean; + destroyError?: Error; + destroy: (err?: Error) => void; +} + +interface HttpsCall { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: any; + req: FakeRequest; + res: EventEmitter; + responseCallback: (res: EventEmitter) => void; +} + +interface FakeHttps { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + httpsStub: Record; + calls: HttpsCall[]; +} + +function makeFakeHttps(): FakeHttps { + const calls: HttpsCall[] = []; + const get = ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + options: any, + responseCallback: (res: EventEmitter) => void + ): FakeRequest => { + const req = new EventEmitter() as FakeRequest; + req.destroyed = false; + req.destroy = (err?: Error) => { + req.destroyed = true; + req.destroyError = err; + if (err) { + // Real Node emits 'error' asynchronously after destroy(err). + process.nextTick(() => req.emit("error", err)); + } + }; + const res = new EventEmitter(); + calls.push({ options, req, res, responseCallback }); + return req; + }; + return { + calls, + httpsStub: { get, "@noCallThru": true } + }; +} + +interface FakeToken { + isCancellationRequested: boolean; + onCancellationRequested: (listener: () => void) => { dispose: () => void }; + _cancel(): void; +} + +function makeFakeToken(initiallyCancelled = false): FakeToken { + const listeners: Array<() => void> = []; + const tok: FakeToken = { + isCancellationRequested: initiallyCancelled, + onCancellationRequested: (listener: () => void) => { + listeners.push(listener); + return { + dispose: () => { + const i = listeners.indexOf(listener); + if (i >= 0) { + listeners.splice(i, 1); + } + } + }; + }, + _cancel: () => { + tok.isCancellationRequested = true; + for (const l of [...listeners]) { + l(); + } + } + }; + return tok; +} + +interface RequestUtilsModule { + getArtifacts: (keywords: string[], token?: FakeToken) => Promise>; + getVersions: (gid: string, aid: string, token?: FakeToken) => Promise>; +} + +function load(): { mod: RequestUtilsModule; calls: HttpsCall[] } { + const { httpsStub, calls } = makeFakeHttps(); + const pq = proxyquire.noPreserveCache(); + const mod = pq("../../src/utils/requestUtils", { + "https": httpsStub, + // requestUtils only imports `vscode` for the CancellationToken type; + // an empty stub is enough to satisfy the require() at runtime. + "vscode": { "@noCallThru": true } + }) as RequestUtilsModule; + return { mod, calls }; +} + +// Yield one macrotask so that the fake https.get gets invoked. +async function flushMicrotasks(): Promise { + await new Promise(resolve => setImmediate(resolve)); +} + +describe("requestUtils — PR #1170", () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let consoleErrorCalls: any[][]; + let originalConsoleError: typeof console.error; + + beforeEach(() => { + consoleErrorCalls = []; + originalConsoleError = console.error; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + console.error = (...args: any[]) => { consoleErrorCalls.push(args); }; + }); + + afterEach(() => { + console.error = originalConsoleError; + }); + + describe("getArtifacts", () => { + it("returns [] without making an HTTPS call when all keywords are too short (< 3 chars)", async () => { + const { mod, calls } = load(); + const result = await mod.getArtifacts(["a", "ab"]); + assert.deepEqual(result, []); + assert.equal(calls.length, 0); + assert.equal(consoleErrorCalls.length, 0); + }); + + it("returns parsed docs on a successful response", async () => { + const { mod, calls } = load(); + const promise = mod.getArtifacts(["junit"]); + + await flushMicrotasks(); + assert.equal(calls.length, 1); + const { res, responseCallback } = calls[0]; + responseCallback(res); + res.emit("data", Buffer.from(JSON.stringify({ + response: { + docs: [{ id: "junit:junit", g: "junit", a: "junit", latestVersion: "4.13.2", versionCount: 60, p: "jar" }] + } + }))); + res.emit("end"); + + const result = await promise; + assert.equal(result.length, 1); + assert.equal(result[0].a, "junit"); + assert.equal(consoleErrorCalls.length, 0); + }); + + it("passes a 10s timeout to https.get", async () => { + const { mod, calls } = load(); + const promise = mod.getArtifacts(["junit"]); + + await flushMicrotasks(); + assert.equal(calls.length, 1); + assert.equal(calls[0].options.timeout, 10_000); + + // Tidy up so the test doesn't leave a pending promise. + const { res, responseCallback } = calls[0]; + responseCallback(res); + res.emit("data", Buffer.from(JSON.stringify({ response: { docs: [] } }))); + res.emit("end"); + await promise; + }); + + it("returns [] without console.error when token is already cancelled (pre-flight short-circuit)", async () => { + const { mod, calls } = load(); + const token = makeFakeToken(true); + + const result = await mod.getArtifacts(["junit"], token); + + assert.deepEqual(result, []); + assert.equal(calls.length, 0, "httpsGet must short-circuit before calling https.get"); + assert.equal(consoleErrorCalls.length, 0, "Pre-cancelled token must not log"); + }); + + it("returns [] without console.error when token is cancelled mid-flight; req is destroyed", async () => { + const { mod, calls } = load(); + const token = makeFakeToken(); + const promise = mod.getArtifacts(["junit"], token); + + await flushMicrotasks(); + assert.equal(calls.length, 1); + + token._cancel(); + const result = await promise; + + assert.deepEqual(result, []); + assert.equal(calls[0].req.destroyed, true, "Cancellation must destroy the in-flight request"); + assert.equal(consoleErrorCalls.length, 0, "Cancellation must not log to console.error"); + }); + + it("returns [] AND logs to console.error on a real network error (non-cancellation)", async () => { + const { mod, calls } = load(); + const promise = mod.getArtifacts(["junit"]); + + await flushMicrotasks(); + calls[0].req.emit("error", new Error("ECONNREFUSED")); + + const result = await promise; + assert.deepEqual(result, []); + assert.equal(consoleErrorCalls.length, 1, "Real failures must still be logged"); + assert.ok(String(consoleErrorCalls[0][0]).includes("ECONNREFUSED")); + }); + + it("on 'timeout' destroys the request with a timeout error and returns []", async () => { + const { mod, calls } = load(); + const promise = mod.getArtifacts(["junit"]); + + await flushMicrotasks(); + const { req } = calls[0]; + + // Simulate Node's socket idle timeout — our handler calls req.destroy(err). + req.emit("timeout"); + + const result = await promise; + assert.deepEqual(result, []); + assert.equal(req.destroyed, true); + assert.ok(req.destroyError, "destroy should be called with an error"); + assert.ok(/timed out/.test(req.destroyError!.message), `unexpected error: ${req.destroyError!.message}`); + assert.equal(consoleErrorCalls.length, 1, "Timeout is a real failure and must be logged"); + }); + }); + + describe("getVersions", () => { + it("returns parsed docs on a successful response", async () => { + const { mod, calls } = load(); + const promise = mod.getVersions("junit", "junit"); + + await flushMicrotasks(); + const { res, responseCallback } = calls[0]; + responseCallback(res); + res.emit("data", Buffer.from(JSON.stringify({ + response: { docs: [{ id: "junit:junit:4.13.2", g: "junit", a: "junit", v: "4.13.2", timestamp: 0 }] } + }))); + res.emit("end"); + + const result = await promise; + assert.equal(result.length, 1); + assert.equal(result[0].v, "4.13.2"); + assert.equal(consoleErrorCalls.length, 0); + }); + + it("returns [] silently when token is cancelled mid-flight", async () => { + const { mod, calls } = load(); + const token = makeFakeToken(); + const promise = mod.getVersions("junit", "junit", token); + + await flushMicrotasks(); + assert.equal(calls.length, 1); + token._cancel(); + + const result = await promise; + assert.deepEqual(result, []); + assert.equal(consoleErrorCalls.length, 0); + }); + }); +}); From b400467e85497c26d80af1b3746dbb50089f8c60 Mon Sep 17 00:00:00 2001 From: wenytang-ms Date: Wed, 27 May 2026 15:37:34 +0800 Subject: [PATCH 4/4] fix(tests): make Node 22 strip-types workaround conditional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runs the unit tests on Node 20, which does not understand the `--no-experimental-strip-types` flag I added in 88eba6a — the previous commit broke `npm run test:unit` on all three CI runners with: bad option: --no-experimental-strip-types Drop the unconditional `node-option` from .mocharc.json and wrap mocha in scripts/run-unit-tests.js, which only forwards the flag on Node 22.6+ (where `--experimental-strip-types` actually exists and is the cause of the harness quirk for newly added .test.ts files). Node 20 runs mocha exactly as it did before this PR. Verified locally on Node 22.21.1: 30 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .mocharc.json | 3 +-- package.json | 2 +- scripts/run-unit-tests.js | 45 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 scripts/run-unit-tests.js diff --git a/.mocharc.json b/.mocharc.json index 7d5f63f1..eb613882 100644 --- a/.mocharc.json +++ b/.mocharc.json @@ -3,6 +3,5 @@ "spec": "test/unit/**/*.test.ts", "extension": ["ts"], "timeout": 15000, - "reporter": "spec", - "node-option": ["no-experimental-strip-types"] + "reporter": "spec" } diff --git a/package.json b/package.json index 87c608b3..fdaeb58b 100644 --- a/package.json +++ b/package.json @@ -825,7 +825,7 @@ "build-plugin": "node scripts/build-jdtls-ext.js", "compile": "tsc -p ./", "tslint": "eslint .", - "test:unit": "mocha", + "test:unit": "node scripts/run-unit-tests.js", "test:autotest": "node scripts/run-autotest-plans.js test-plans", "watch": "webpack --mode development --watch --progress", "webpack": "webpack --mode development", diff --git a/scripts/run-unit-tests.js b/scripts/run-unit-tests.js new file mode 100644 index 00000000..02483127 --- /dev/null +++ b/scripts/run-unit-tests.js @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +/** + * Wrapper that invokes mocha for the unit-test suite. + * + * Why this exists: + * Node 22.6+ ships `--experimental-strip-types` (on by default since 22.18). + * When that flag is enabled, Mocha can load newly added `test/unit/*.test.ts` + * files through Node's native ESM strip-types loader instead of the + * `ts-node/register/transpile-only` CommonJS hook configured in + * `.mocharc.json`. That breaks any test file that uses `require()` + * (e.g. for `proxyquire`) with: + * ReferenceError: require is not defined in ES module scope + * + * The fix is to pass `--no-experimental-strip-types` to Node so that all + * `.ts` test files load uniformly via ts-node. But that flag does not + * exist on Node 20, where the project's CI still runs, so we cannot put + * it directly in `.mocharc.json` — Node 20 would reject it as + * `bad option`. + * + * This wrapper detects the Node version at runtime and only forwards + * the flag on Node 22.6+. Node 20 runs mocha exactly as before. + */ + +"use strict"; + +const { spawn } = require("child_process"); + +const [major, minor] = process.versions.node.split(".").map(Number); +const supportsStripTypesFlag = major > 22 || (major === 22 && minor >= 6); + +const nodeArgs = []; +if (supportsStripTypesFlag) { + nodeArgs.push("--no-experimental-strip-types"); +} +nodeArgs.push(require.resolve("mocha/bin/mocha.js")); +nodeArgs.push(...process.argv.slice(2)); + +const child = spawn(process.execPath, nodeArgs, { stdio: "inherit" }); +child.on("close", code => process.exit(code ?? 1)); +child.on("error", err => { + console.error(err); + process.exit(1); +});