Skip to content

Clean up WaitForSessionFile logic and support increasing timeout with warning #2653

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.

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
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,11 @@
"default": null,
"description": "An array of strings that enable experimental features in the PowerShell extension."
},
"powershell.developer.waitForSessionFileNumOfTries": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a setting I think it would make more sense to expose a timeout and then in the implementation just retry until the total exceeds the timeout

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that. Perhaps powershell.developer.sessionStartupTimeout?

"type":"number",
"default": 120,
"description": "When the PowerShell extension is starting up, it checks for a session file in order to connect to the language server. It checks this number of times ever 2 seconds."
},
"powershell.pester.useLegacyCodeLens": {
"type": "boolean",
"default": true,
Expand Down
38 changes: 27 additions & 11 deletions src/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,36 @@ export class PowerShellProcess {
return true;
}

private waitForSessionFile(): Promise<utils.IEditorServicesSessionDetails> {
return new Promise((resolve, reject) => {
utils.waitForSessionFile(this.sessionFilePath, (sessionDetails, error) => {
utils.deleteSessionFile(this.sessionFilePath);
private sleep(ms: number) {
return new Promise(resolve => setTimeout(resolve, ms));
}

private async waitForSessionFile(): Promise<utils.IEditorServicesSessionDetails> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! We've been using async/await in our Electron/Angular app code base and vastly prefer it to dealing with Promises directly. :-)

const numOfTries = this.sessionSettings.developer.waitForSessionFileNumOfTries;

if (error) {
this.log.write(`Error occurred retrieving session file:\n${error}`);
return reject(error);
}
// This is used to warn the user that the extension is taking longer than expected to startup.
const warnUserThreshold = numOfTries / 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the warning, I'd imagine our standard expectation for startup time is a constant, like 30 seconds, rather than parameterised by the setting


for (let i = numOfTries; i > 0; i--) {
if (utils.checkIfFileExists(this.sessionFilePath)) {
this.log.write("Session file found");
resolve(sessionDetails);
});
});
const sessionDetails = utils.readSessionFile(this.sessionFilePath);
utils.deleteSessionFile(this.sessionFilePath);
return sessionDetails;
}

if (warnUserThreshold === i) {
vscode.window.showWarningMessage(`Loading the PowerShell extension is taking longer than expected.
If you're using anti-virus software, this can affect start up performance.`);
}

// Wait a bit and try again
await this.sleep(2000);
}

const err = "Timed out waiting for session file to appear.";
this.log.write(err);
throw new Error(err);
}

private onTerminalClose(terminal: vscode.Terminal) {
Expand Down
2 changes: 2 additions & 0 deletions src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export interface IDeveloperSettings {
bundledModulesPath?: string;
editorServicesLogLevel?: string;
editorServicesWaitForDebugger?: boolean;
waitForSessionFileNumOfTries?: number;
}

export interface ISettings {
Expand Down Expand Up @@ -142,6 +143,7 @@ export function load(): ISettings {
bundledModulesPath: "../../../PowerShellEditorServices/module",
editorServicesLogLevel: "Normal",
editorServicesWaitForDebugger: false,
waitForSessionFileNumOfTries: 120,
};

const defaultCodeFoldingSettings: ICodeFoldingSettings = {
Expand Down
19 changes: 0 additions & 19 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,6 @@ export function writeSessionFile(sessionFilePath: string, sessionDetails: IEdito
writeStream.close();
}

export function waitForSessionFile(sessionFilePath: string, callback: IWaitForSessionFileCallback) {

function innerTryFunc(remainingTries: number, delayMilliseconds: number) {
if (remainingTries === 0) {
callback(undefined, "Timed out waiting for session file to appear.");
} else if (!checkIfFileExists(sessionFilePath)) {
// Wait a bit and try again
setTimeout(
() => { innerTryFunc(remainingTries - 1, delayMilliseconds); },
delayMilliseconds);
} else {
// Session file was found, load and return it
callback(readSessionFile(sessionFilePath), undefined);
}
}

// Try once every 2 seconds, 60 times - making two full minutes
innerTryFunc(60, 2000);
}

export function readSessionFile(sessionFilePath: string): IEditorServicesSessionDetails {
const fileContents = fs.readFileSync(sessionFilePath, "utf-8");
Expand Down