Skip to content

Commit eaa5931

Browse files
Handle version-details timeout and stalled connects during startup
Two latent startup-path bugs in `src/session.ts`, surfaced while investigating #5031 (neither caused it): - `getVersionDetails()` set a 60s timeout that only cancelled the token, so when it fired `sendRequest(...)` rejected and the `await` threw straight out of `start()` — the intended graceful failure handling was never reached. We now catch the rejection, log the underlying cause as a warning, and return `undefined` so the existing failure branch runs and surfaces a friendlier, more actionable message to the user. While here, fix the timer leak: we capture the `setTimeout` handle to `clearTimeout` it and `dispose()` the `CancellationTokenSource`. - `connectFunc` had no error or timeout handler, so a transport that never connected just hung with nothing actionable. We add an `error` handler and a 60s connect timeout that destroys the socket and rejects with a clear, logged message naming the pipe. Both paths clear the timer so a healthy but idle connection is never torn down. Adds unit tests for `getVersionDetails()` covering the reject-to- `undefined` path and the normal fast path. `connectFunc` is a private closure over a live `net` socket with no test seam, so I left it uncovered rather than add socket-mocking infra the repo deliberately avoids. Closes #5521. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7884b75 commit eaa5931

2 files changed

Lines changed: 143 additions & 12 deletions

File tree

‎src/session.ts‎

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ export class SessionManager implements Middleware {
280280
this.versionDetails = await this.getVersionDetails();
281281
if (this.versionDetails === undefined) {
282282
void this.setSessionFailedOpenBug(
283-
"Unable to get version details!",
283+
"PowerShell started but didn't report its version in time, so the session couldn't finish starting up!",
284284
);
285285
return;
286286
}
@@ -849,14 +849,31 @@ export class SessionManager implements Middleware {
849849
): Promise<LanguageClient> {
850850
this.logger.writeDebug("Connecting to language service...");
851851
const connectFunc = (): Promise<StreamInfo> => {
852-
return new Promise<StreamInfo>((resolve, _reject) => {
853-
const socket = net.connect(
854-
sessionDetails.languageServicePipeName,
855-
);
852+
return new Promise<StreamInfo>((resolve, reject) => {
853+
const pipeName = sessionDetails.languageServicePipeName;
854+
const socket = net.connect(pipeName);
855+
856+
// Reject (rather than hang forever) if the transport never
857+
// connects, e.g. a stalled named pipe or socket.
858+
const timeout = setTimeout(() => {
859+
const message = `Timed out connecting to language service at '${pipeName}'!`;
860+
this.logger.writeError(message);
861+
socket.destroy();
862+
reject(new Error(message));
863+
}, 60 * 1000);
864+
856865
socket.on("connect", () => {
866+
clearTimeout(timeout);
857867
this.logger.writeDebug("Language service connected.");
858868
resolve({ writer: socket, reader: socket });
859869
});
870+
871+
socket.on("error", (error) => {
872+
clearTimeout(timeout);
873+
const message = `Error connecting to language service at '${pipeName}': ${error.message}`;
874+
this.logger.writeError(message);
875+
reject(error);
876+
});
860877
});
861878
};
862879

@@ -1070,16 +1087,34 @@ Type 'help' to get help.
10701087
private async getVersionDetails(): Promise<
10711088
IPowerShellVersionDetails | undefined
10721089
> {
1073-
// Take one minute to get version details, otherwise cancel and fail.
1090+
// Take one minute to get version details, otherwise cancel and fail
1091+
// gracefully by returning undefined rather than throwing out of
1092+
// start(). The caller handles undefined by surfacing the startup
1093+
// failure to the user.
10741094
const timeout = new vscode.CancellationTokenSource();
1075-
setTimeout(() => {
1095+
const timer = setTimeout(() => {
10761096
timeout.cancel();
10771097
}, 60 * 1000);
10781098

1079-
const versionDetails = await this.languageClient?.sendRequest(
1080-
PowerShellVersionRequestType,
1081-
timeout.token,
1082-
);
1099+
let versionDetails: IPowerShellVersionDetails | undefined;
1100+
try {
1101+
versionDetails = await this.languageClient?.sendRequest(
1102+
PowerShellVersionRequestType,
1103+
timeout.token,
1104+
);
1105+
} catch (err) {
1106+
// A cancelled token (our timeout elapsed) or any other transport
1107+
// failure rejects the request. Swallow it and return undefined so
1108+
// the caller's failure handling runs instead of throwing. We log
1109+
// the underlying cause here since the caller's error won't include
1110+
// it; the caller surfaces the user-facing message.
1111+
this.logger.writeWarning(
1112+
`The PowerShell version request did not complete: ${err}`,
1113+
);
1114+
} finally {
1115+
clearTimeout(timer);
1116+
timeout.dispose();
1117+
}
10831118

10841119
// This is pretty much the only telemetry event we care about.
10851120
// TODO: We actually could send this earlier from PSES itself.

‎test/core/session.test.ts‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import * as assert from "assert";
66
import Sinon from "sinon";
77
import * as vscode from "vscode";
88
import type { DocumentSelector } from "vscode-languageclient";
9-
import { SessionManager } from "../../src/session";
9+
import {
10+
type IPowerShellVersionDetails,
11+
SessionManager,
12+
} from "../../src/session";
1013
import { stubInterface, testLogger } from "../utils";
1114

1215
describe("SessionManager middleware", () => {
@@ -163,6 +166,99 @@ describe("SessionManager middleware", () => {
163166
});
164167
});
165168

169+
describe("SessionManager.getVersionDetails", () => {
170+
afterEach(() => {
171+
Sinon.restore();
172+
});
173+
174+
it("returns undefined instead of throwing when the request rejects", async () => {
175+
const manager = makeManager();
176+
// Simulate the request rejecting, as it does when our timeout cancels
177+
// the token. This must not throw out of getVersionDetails().
178+
setSendRequest(manager, () => Promise.reject(new Error("Canceled")));
179+
180+
const result = await getVersionDetails(manager);
181+
assert.strictEqual(
182+
result,
183+
undefined,
184+
"a rejected (e.g. timed-out/cancelled) request should resolve to undefined",
185+
);
186+
});
187+
188+
it("returns the details on the normal fast path", async () => {
189+
const manager = makeManager();
190+
const details: IPowerShellVersionDetails = {
191+
version: "7.4.0",
192+
edition: "Core",
193+
commit: "7.4.0",
194+
architecture: "X64",
195+
};
196+
setSendRequest(manager, () => Promise.resolve(details));
197+
198+
const result = await getVersionDetails(manager);
199+
assert.strictEqual(result, details);
200+
});
201+
});
202+
203+
function makeManager(): SessionManager {
204+
Sinon.stub(vscode.commands, "registerCommand").returns(disposableStub());
205+
Sinon.stub(vscode.workspace, "onDidChangeConfiguration").returns(
206+
disposableStub(),
207+
);
208+
Sinon.stub(vscode.languages, "createLanguageStatusItem").returns(
209+
stubInterface<vscode.LanguageStatusItem>({
210+
text: "",
211+
detail: "",
212+
busy: false,
213+
severity: vscode.LanguageStatusSeverity.Information,
214+
dispose: () => {
215+
return;
216+
},
217+
}),
218+
);
219+
220+
return new SessionManager(
221+
stubInterface<vscode.ExtensionContext>({
222+
globalStorageUri: vscode.Uri.file("C:/tmp"),
223+
extensionMode: vscode.ExtensionMode.Test,
224+
subscriptions: [],
225+
logUri: vscode.Uri.file("C:/tmp"),
226+
}),
227+
testLogger,
228+
["powershell"] as DocumentSelector,
229+
"Visual Studio Code",
230+
"PowerShell",
231+
"2026.5.0",
232+
"ms-vscode",
233+
stubInterface<TelemetryReporter>(),
234+
);
235+
}
236+
237+
function setSendRequest(
238+
manager: SessionManager,
239+
sendRequest: () => Promise<IPowerShellVersionDetails | undefined>,
240+
): void {
241+
(
242+
manager as unknown as {
243+
languageClient: {
244+
sendRequest: () => Promise<
245+
IPowerShellVersionDetails | undefined
246+
>;
247+
};
248+
}
249+
).languageClient = { sendRequest };
250+
}
251+
252+
function getVersionDetails(
253+
manager: SessionManager,
254+
): Promise<IPowerShellVersionDetails | undefined> {
255+
return (
256+
manager as unknown as {
257+
getVersionDetails(): Promise<IPowerShellVersionDetails | undefined>;
258+
}
259+
).getVersionDetails();
260+
}
261+
166262
function disposableStub(): vscode.Disposable {
167263
return {
168264
dispose: (): void => {

0 commit comments

Comments
 (0)