Skip to content

Commit

Permalink
Fix lsp missing crash (#594)
Browse files Browse the repository at this point in the history
* Fix crashes where workspaceFolders was undefined

* Fix fallback to built-in lsp when bsdk cannot be resolved

* Better error message wording
  • Loading branch information
TwitchBronBron authored Sep 26, 2024
1 parent 79c8964 commit e89413f
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 4 deletions.
22 changes: 22 additions & 0 deletions src/LanguageServerManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { util } from './util';
import { GlobalStateManager } from './GlobalStateManager';
import { LocalPackageManager } from './managers/LocalPackageManager';
import { expectThrowsAsync } from './testHelpers.spec';
const Module = require('module');
const sinon = createSandbox();

Expand Down Expand Up @@ -345,6 +346,27 @@ describe('LanguageServerManager', () => {
//these tests take a long time (due to running `npm install`)
this.timeout(5 * 60 * 1000);

it('returns info when absolute dir already exists', async () => {
const versionInfo = s`${tempDir}/node_modules/brighterscript`;
fsExtra.outputJsonSync(s`${tempDir}/node_modules/brighterscript/package.json`, {
version: '0.65.0'
});
expect(
await languageServerManager['ensureBscVersionInstalled'](versionInfo)
).to.eql({
packageDir: s`${tempDir}/node_modules/brighterscript`,
versionInfo: versionInfo,
version: '0.65.0'
});
});

it('throws when dir does not exist', async () => {
const versionInfo = s`${tempDir}/node_modules/brighterscript`;
await expectThrowsAsync(async () => {
await languageServerManager['ensureBscVersionInstalled'](versionInfo);
}, `"${s(versionInfo)}" does not contain a package.json file`);
});

it('installs a bsc version when not present', async () => {
const info = await languageServerManager['ensureBscVersionInstalled']('0.65.0');
expect(info).to.eql({
Expand Down
12 changes: 8 additions & 4 deletions src/LanguageServerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ export class LanguageServerManager {
this.selectedBscInfo = await this.ensureBscVersionInstalled(versionInfo);
} catch (e) {
console.error(e);
//fall back to the embedded version, and show a popup
await vscode.window.showErrorMessage(`Can't find language server for "${versionInfo}". Did you forget to run \`npm install\`? Using embedded version v${this.embeddedBscInfo.version} instead.`);
//fall back to the embedded version, and show a popup (don't await the popup because that blocks this flow)
void vscode.window.showErrorMessage(`Language server failure. Did you forget \`npm install\`? Using embedded version ${this.embeddedBscInfo.version}. Can't find language server for "${versionInfo}"`);
this.selectedBscInfo = this.embeddedBscInfo;
}

Expand All @@ -439,7 +439,7 @@ export class LanguageServerManager {
if (versionInfo === 'embedded') {
return {
type: 'dir',
value: this.embeddedBscInfo.packageDir
value: s`${this.embeddedBscInfo.packageDir}`
};
} else {
return this.localPackageManager.parseVersionInfo(versionInfo, cwd);
Expand Down Expand Up @@ -513,8 +513,12 @@ export class LanguageServerManager {

//if this is a directory, use it as-is
if (parsed.type === 'dir') {
//if the directory does not exist, throw an error
if (await fsExtra.pathExists(s`${parsed.value}/package.json`) === false) {
throw new Error(`"${parsed.value}" does not contain a package.json file`);
}
return {
packageDir: parsed.value,
packageDir: s`${parsed.value}`,
version: fsExtra.readJsonSync(s`${parsed.value}/package.json`, { throws: false })?.version ?? parsed.value,
versionInfo: versionInfo
};
Expand Down
32 changes: 32 additions & 0 deletions src/testHelpers.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { expect } from 'chai';

export function expectThrows(callback: () => any, expectedMessage: string | undefined = undefined, failedTestMessage = 'Expected to throw but did not') {
let wasExceptionThrown = false;
try {
callback();
} catch (e) {
wasExceptionThrown = true;
if (expectedMessage) {
expect(e.message).to.eql(expectedMessage);
}
}
if (wasExceptionThrown === false) {
throw new Error(failedTestMessage);
}
}


export async function expectThrowsAsync(callback: () => any, expectedMessage: string | undefined = undefined, failedTestMessage = 'Expected to throw but did not') {
let wasExceptionThrown = false;
try {
await Promise.resolve(callback());
} catch (e) {
wasExceptionThrown = true;
if (expectedMessage) {
expect(e.message).to.eql(expectedMessage);
}
}
if (wasExceptionThrown === false) {
throw new Error(failedTestMessage);
}
}

0 comments on commit e89413f

Please sign in to comment.