Skip to content

Update startup logic to handle session failure reasons #4543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 25, 2023
Merged

Update startup logic to handle session failure reasons #4543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
251c495
Update startup logic to handle session failure reasons
andyleejordan Apr 24, 2023
686b4c9
Fix "Show Session Menu" to use updated settings
andyleejordan Apr 24, 2023
b0f85f3
Fix handling the Extension Terminal failing to start
andyleejordan Apr 24, 2023
2d74846
Get version for status icon when PowerShell fails to start
andyleejordan Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions src/process.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,28 @@ export class PowerShellProcess {
hideFromUser: this.sessionSettings.integratedConsole.startInBackground,
};

// Subscribe a log event for when the terminal closes (this fires for
// all terminals and the event itself checks if it's our terminal). This
// subscription should happen before we create the terminal so if it
// fails immediately, the event fires.
this.consoleCloseSubscription = vscode.window.onDidCloseTerminal((terminal) => this.onTerminalClose(terminal));

this.consoleTerminal = vscode.window.createTerminal(terminalOptions);

const pwshName = path.basename(this.exePath);
this.logger.write(`${pwshName} started.`);

// Log that the PowerShell terminal process has been started
const pid = await this.getPid();
this.logTerminalPid(pid ?? 0, pwshName);

if (this.sessionSettings.integratedConsole.showOnStartup
&& !this.sessionSettings.integratedConsole.startInBackground) {
// We still need to run this to set the active terminal to the extension terminal.
this.consoleTerminal.show(true);
}

// Start the language client
const sessionDetails = await this.waitForSessionFile();

// Subscribe a log event for when the terminal closes
this.consoleCloseSubscription = vscode.window.onDidCloseTerminal((terminal) => this.onTerminalClose(terminal));

// Log that the PowerShell terminal process has been started
const pid = await this.getPid();
this.logTerminalPid(pid ?? 0, pwshName);

return sessionDetails;
return await this.waitForSessionFile();
}

// This function should only be used after a failure has occurred because it is slow!
Expand All @@ -154,7 +154,7 @@ export class PowerShellProcess {

public async dispose(): Promise<void> {
// Clean up the session file
this.logger.write("Terminating PowerShell process...");
this.logger.write("Disposing PowerShell Extension Terminal...");

this.consoleTerminal?.dispose();
this.consoleTerminal = undefined;
Expand Down Expand Up @@ -199,8 +199,8 @@ export class PowerShellProcess {
private static async deleteSessionFile(sessionFilePath: vscode.Uri): Promise<void> {
try {
await vscode.workspace.fs.delete(sessionFilePath);
} catch (e) {
// TODO: Be more specific about what we're catching
} catch {
// We don't care about any reasons for it to fail.
}
}

Expand All @@ -212,6 +212,12 @@ export class PowerShellProcess {
// Check every 2 seconds
this.logger.write("Waiting for session file...");
for (let i = numOfTries; i > 0; i--) {
if (this.consoleTerminal === undefined) {
const err = "PowerShell Extension Terminal didn't start!";
this.logger.write(err);
throw new Error(err);
}

if (await utils.checkIfFileExists(this.sessionFilePath)) {
this.logger.write("Session file found!");
const sessionDetails = await PowerShellProcess.readSessionFile(this.sessionFilePath);
Expand All @@ -237,7 +243,8 @@ export class PowerShellProcess {
return;
}

this.logger.write("powershell.exe terminated or terminal UI was closed");
this.logger.write("PowerShell process terminated or Extension Terminal was closed!");
this.onExitedEmitter.fire();
void this.dispose();
}
}
36 changes: 21 additions & 15 deletions src/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export enum RunspaceType {
export interface IEditorServicesSessionDetails {
status: string;
reason: string;
detail: string;
powerShellVersion: string;
channel: string;
languageServicePort: number;
Expand Down Expand Up @@ -430,7 +429,7 @@ export class SessionManager implements Middleware {

private async startPowerShell(): Promise<PowerShellProcess | undefined> {
if (this.PowerShellExeDetails === undefined) {
this.setSessionFailure("Unable to find PowerShell.");
void this.setSessionFailedGetPowerShell("Unable to find PowerShell, try installing it?");
return;
}

Expand Down Expand Up @@ -458,9 +457,6 @@ export class SessionManager implements Middleware {
try {
this.sessionDetails = await languageServerProcess.start("EditorServices");
} catch (err) {
// We should kill the process in case it's stuck.
void languageServerProcess.dispose();

// PowerShell never started, probably a bad version!
const version = await languageServerProcess.getVersionCli();
let shouldUpdate = true;
Expand Down Expand Up @@ -498,10 +494,10 @@ export class SessionManager implements Middleware {
void this.setSessionFailedOpenBug("Language client failed to start: " + (err instanceof Error ? err.message : "unknown"));
}
} else if (this.sessionDetails.status === "failed") { // Server started but indicated it failed
if (this.sessionDetails.reason === "unsupported") {
if (this.sessionDetails.reason === "powerShellVersion") {
void this.setSessionFailedGetPowerShell(`PowerShell ${this.sessionDetails.powerShellVersion} is not supported, please update!`);
} else if (this.sessionDetails.reason === "languageMode") {
this.setSessionFailure(`PowerShell language features are disabled due to an unsupported LanguageMode: ${this.sessionDetails.detail}`);
} else if (this.sessionDetails.reason === "dotNetVersion") { // Only applies to PowerShell 5.1
void this.setSessionFailedGetDotNet(".NET Framework is out-of-date, please install at least 4.8!");
} else {
void this.setSessionFailedOpenBug(`PowerShell could not be started for an unknown reason: ${this.sessionDetails.reason}`);
}
Expand Down Expand Up @@ -728,7 +724,7 @@ Type 'help' to get help.
try {
await this.languageClient.start();
} catch (err) {
this.setSessionFailure("Could not start language service: ", err instanceof Error ? err.message : "unknown");
void this.setSessionFailedOpenBug("Could not start language service: " + (err instanceof Error ? err.message : "unknown"));
return;
}

Expand Down Expand Up @@ -761,6 +757,9 @@ Type 'help' to get help.
const semver = new SemVer(this.versionDetails.version);
this.languageStatusItem.text = `$(terminal-powershell) ${semver.major}.${semver.minor}`;
this.languageStatusItem.detail += ` ${this.versionDetails.commit} (${this.versionDetails.architecture.toLowerCase()})`;
} else if (this.PowerShellExeDetails?.displayName) { // In case it didn't start.
this.languageStatusItem.text = `$(terminal-powershell) ${this.PowerShellExeDetails.displayName}`;
this.languageStatusItem.detail += `: ${this.PowerShellExeDetails.exePath}`;
}

if (statusText) {
Expand Down Expand Up @@ -799,11 +798,6 @@ Type 'help' to get help.
this.setSessionStatus("Executing...", SessionStatus.Busy);
}

private setSessionFailure(message: string, ...additionalMessages: string[]): void {
this.setSessionStatus("Initialization Error!", SessionStatus.Failed);
void this.logger.writeAndShowError(message, ...additionalMessages);
}

private async setSessionFailedOpenBug(message: string): Promise<void> {
this.setSessionStatus("Initialization Error!", SessionStatus.Failed);
await this.logger.writeAndShowErrorWithActions(message, [{
Expand All @@ -825,6 +819,17 @@ Type 'help' to get help.
);
}

private async setSessionFailedGetDotNet(message: string): Promise<void> {
this.setSessionStatus("Initialization Error!", SessionStatus.Failed);
await this.logger.writeAndShowErrorWithActions(message, [{
prompt: "Open .NET Framework Documentation",
action: async (): Promise<void> => {
await vscode.env.openExternal(
vscode.Uri.parse("https://dotnet.microsoft.com/en-us/download/dotnet-framework"));
}}]
);
}

private async changePowerShellDefaultVersion(exePath: IPowerShellExeDetails): Promise<void> {
this.suppressRestartPrompt = true;
try {
Expand Down Expand Up @@ -857,7 +862,8 @@ Type 'help' to get help.
private async showSessionMenu(): Promise<void> {
const powershellExeFinder = new PowerShellExeFinder(
this.platformDetails,
this.sessionSettings.powerShellAdditionalExePaths,
// We don't pull from session settings because we want them fresh!
getSettings().powerShellAdditionalExePaths,
this.logger);
const availablePowerShellExes = await powershellExeFinder.getAllAvailablePowerShellInstallations();

Expand Down