Skip to content
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

Fix some flakiness caused by PR #957 #967

Merged
merged 2 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 8 additions & 6 deletions packages/extension/src/language-client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ export class SQLToolsLanguageClient implements ILanguageClient {

private avoidRestart = false;
constructor() {
this.onNotification(ExitCalledNotification, () => {
this.avoidRestart = true;
});

this.registerBaseNotifications();

Config.addOnUpdateHook(async ({ event }) => {
if (event.affectsConfig('useNodeRuntime')) {
const res = await window.showWarningMessage('Use node runtime setting change. You must reload window to take effect.', 'Reload now');
Expand Down Expand Up @@ -63,8 +57,14 @@ export class SQLToolsLanguageClient implements ILanguageClient {
return defaultErrorHandler.closed();
},
};

this.onNotification(ExitCalledNotification, () => {
this.avoidRestart = true;
});

this.registerBaseNotifications();
}

public start() {
return this.client.start();
}
Expand All @@ -83,6 +83,7 @@ export class SQLToolsLanguageClient implements ILanguageClient {
await this.client.onReady();
return this.client.sendNotification.apply(this.client, arguments);
}

public onNotification: LanguageClient['onNotification'] = async function () {
await this.client.onReady();
return this.client.onNotification.apply(this.client, arguments);
Expand All @@ -100,6 +101,7 @@ export class SQLToolsLanguageClient implements ILanguageClient {
runtime = runtimePath;
}
} else {
log.info('Detecting node path (if this stalls check Terminal view for the stuck session and kill it)...');
const nodePath = await detectNodePath();
if (nodePath) {
const message = `Node runtime auto-detected. Using ${nodePath}.`;
Expand Down
5 changes: 3 additions & 2 deletions packages/extension/src/language-client/detect-node-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ const nodeRuntimeTmpFile = getDataPath(".node-runtime");
const detectNodePath = async (): Promise<string | null> => {
try {
const terminal = window.createTerminal({ name: "detect node runtime" });
await new Promise<void>((resolve) => {
const shellExitCommand = await getShellExitCommand();
await new Promise<void>(async (resolve) => {
window.onDidCloseTerminal((e => e.processId === terminal.processId && resolve()));
const nodeCmd = `require("fs").writeFileSync("${nodeRuntimeTmpFile}", process.execPath)`;
const nodeCmdWindows = nodeCmd.replace(/\\/g, '\\\\').replace(/\"/g, '\\"');
terminal.sendText(`node -e '${process.platform === 'win32' ? nodeCmdWindows : nodeCmd}' && ${getShellExitCommand()}`);
terminal.sendText(`node -e '${process.platform === 'win32' ? nodeCmdWindows : nodeCmd}' && ${shellExitCommand}`);
})
return fs.readFileSync(nodeRuntimeTmpFile).toString();
} catch (error) {
Expand Down
20 changes: 18 additions & 2 deletions packages/vscode/utils/get-shell-exit-cmd.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,23 @@
import { env } from 'vscode';

export default function getShellExitCommand(code = 0) {
const isPowerShell = env.shell.match(/[\\/]pwsh(\.exe)?$/g);
export default async function getShellExitCommand(code = 0) {

// env.shell in 1.71 sometimes returns '' unexpectedly.
mtxr marked this conversation as resolved.
Show resolved Hide resolved
// Use a retry loop to try and overcome this.
let shell = '';
for (let attempt = 1; attempt <= 10; attempt++) {
shell = env.shell;
if (shell !== '') {
break;
}
await new Promise(c => setTimeout(c, 500));
}

if (shell === '') {
throw new Error('No env.shell retrieved despite retries');
}

const isPowerShell = shell.match(/[\\/]pwsh(\.exe)?$/g);
if (isPowerShell) return `$(exit ${code})`;
return `exit ${code}`;
}