Skip to content

Commit

Permalink
Fix lsp crash tracker (#576)
Browse files Browse the repository at this point in the history 
* Fix bug with lsp crash tracker

* Add unit tests, handle a few error flows
  • Loading branch information
@TwitchBronBron
TwitchBronBron authored Jun 4, 2024
1 parent 1f7b932 commit 3a92232
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 77 deletions.
70 changes: 61 additions & 9 deletions src/LanguageServerManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ import { DefinitionRepository } from './DefinitionRepository';
import { DeclarationProvider } from './DeclarationProvider';
import type { ExtensionContext } from 'vscode';
import * as path from 'path';
import { standardizePath as s } from 'brighterscript';
import { Deferred, standardizePath as s } from 'brighterscript';
import * as fsExtra from 'fs-extra';
import URI from 'vscode-uri';
import { languageServerInfoCommand } from './commands/LanguageServerInfoCommand';

import type { StateChangeEvent } from 'vscode-languageclient/node';
import {
LanguageClient,
State
} from 'vscode-languageclient/node';
const Module = require('module');
const sinon = createSandbox();

Expand All @@ -37,24 +41,71 @@ describe('LanguageServerManager', () => {
new DeclarationProvider()
);
languageServerManager['context'] = {
asAbsolutePath: vscode.context.asAbsolutePath,
subscriptions: [],
asAbsolutePath: () => { },
globalState: {
get: () => {

},
update: () => {

}
get: () => { },
update: () => { }
}
} as unknown as ExtensionContext;
});

function stubConstructClient(processor?: (LanguageClient) => void) {
sinon.stub(languageServerManager as any, 'constructLanguageClient').callsFake(() => {
const client = {
start: () => { },
onDidChangeState: (cb) => {
},
onReady: () => Promise.resolve(),
onNotification: () => { }
};
processor?.(client);
return client;
});
}

afterEach(() => {
sinon.restore();
fsExtra.removeSync(tempDir);
});

describe('lsp crash tracking', () => {
it('shows popup after a stop without a subsequent start/restart/running', async () => {
let changeState: (event: StateChangeEvent) => void;
//disable starting so we can manually test
sinon.stub(languageServerManager, 'syncVersionAndTryRun').callsFake(() => Promise.resolve());

await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']);

languageServerManager['lspRunTracker'].debounceDelay = 100;

let registerOnDidChangeStateDeferred = new Deferred();
stubConstructClient((client) => {
client.onDidChangeState = (cb) => {
changeState = cb as unknown as any;
registerOnDidChangeStateDeferred.resolve();
};
});

void languageServerManager['enableLanguageServer']();

await registerOnDidChangeStateDeferred.promise;
let showErrorMessageDeferred = new Deferred();
sinon.stub(vscode.window, 'showErrorMessage').callsFake(() => {
showErrorMessageDeferred.resolve();
});

//call the callback with the stopped state
changeState({
oldState: State.Stopped,
newState: State.Stopped
});

// the test will fail if the error message not shown
await showErrorMessageDeferred.promise;
});
});

describe('updateStatusbar', () => {
it('does not crash when undefined', () => {
delete languageServerManager['statusbarItem'];
Expand Down Expand Up @@ -94,6 +145,7 @@ describe('LanguageServerManager', () => {

describe('enableLanguageServer', () => {
it('properly handles runtime exception', async () => {
stubConstructClient();
languageServerManager['client'] = {} as any;
sinon.stub(languageServerManager as any, 'ready').callsFake(() => {
throw new Error('failed for test');
Expand Down
122 changes: 68 additions & 54 deletions src/LanguageServerManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,64 @@ export class LanguageServerManager {

private clientDispose: Disposable;

/**
* Create a new LanguageClient instance
* @returns
*/
private constructLanguageClient() {

// The server is implemented in node
let serverModule = this.context.asAbsolutePath(
path.join('dist', 'LanguageServerRunner.js')
);

//give the runner the specific version of bsc to run
const args = [
this.selectedBscInfo.path,
(this.context.extensionMode === vscode.ExtensionMode.Development).toString()
];
// If the extension is launched in debug mode then the debug server options are used
// Otherwise the run options are used
let serverOptions: ServerOptions = {
run: {
module: serverModule,
transport: TransportKind.ipc,
args: args
},
debug: {
module: serverModule,
transport: TransportKind.ipc,
args: args,
// --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging
options: { execArgv: ['--nolazy', '--inspect=6009'] }
}
};

// Options to control the language client
let clientOptions: LanguageClientOptions = {
// Register the server for various types of documents
documentSelector: [
{ scheme: 'file', language: 'brightscript' },
{ scheme: 'file', language: 'brighterscript' },
{ scheme: 'file', language: 'xml' }
],
synchronize: {
// Notify the server about file changes to every filetype it cares about
fileEvents: workspace.createFileSystemWatcher('**/*')
}
};

// Create the language client and start the client.
return new LanguageClient(
'brighterScriptLanguageServer',
LANGUAGE_SERVER_NAME,
serverOptions,
clientOptions
);
}

private async enableLanguageServer() {
try {

//if we already have a language server, nothing more needs to be done
if (this.client) {
return await this.ready();
Expand All @@ -168,75 +223,34 @@ export class LanguageServerManager {
//disable the simple providers (the language server will handle all of these)
this.disableSimpleProviders();

// The server is implemented in node
let serverModule = this.context.asAbsolutePath(
path.join('dist', 'LanguageServerRunner.js')
);

//give the runner the specific version of bsc to run
const args = [
this.selectedBscInfo.path,
(this.context.extensionMode === vscode.ExtensionMode.Development).toString()
];
// If the extension is launched in debug mode then the debug server options are used
// Otherwise the run options are used
let serverOptions: ServerOptions = {
run: {
module: serverModule,
transport: TransportKind.ipc,
args: args
},
debug: {
module: serverModule,
transport: TransportKind.ipc,
args: args,
// --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging
options: { execArgv: ['--nolazy', '--inspect=6009'] }
}
};

// Options to control the language client
let clientOptions: LanguageClientOptions = {
// Register the server for various types of documents
documentSelector: [
{ scheme: 'file', language: 'brightscript' },
{ scheme: 'file', language: 'brighterscript' },
{ scheme: 'file', language: 'xml' }
],
synchronize: {
// Notify the server about file changes to every filetype it cares about
fileEvents: workspace.createFileSystemWatcher('**/*')
}
};
this.client = this.constructLanguageClient();

// Create the language client and start the client.
this.client = new LanguageClient(
'brighterScriptLanguageServer',
LANGUAGE_SERVER_NAME,
serverOptions,
clientOptions
);
// Start the client. This will also launch the server
this.clientDispose = this.client.start();
await this.client.onReady();
this.client.onDidChangeState((event: StateChangeEvent) => {
console.log(new Date().toLocaleTimeString(), 'onDidChangeState', State[event.newState]);
this.lspRunTracker.setState(event.newState);
});

// Start the client. This will also launch the server
this.clientDispose = this.client.start();

await this.client.onReady();

this.client.onNotification('critical-failure', (message) => {
void window.showErrorMessage(message);
});
this.registerBusyStatusHandler();
this.deferred.resolve(true);
} catch (e) {
console.error(e);
void this.client?.stop?.();
//stop the client by any means necessary
try {
void this.client?.stop?.();
} catch { }
delete this.client;

this.refreshDeferred();

this.deferred.reject(e);
this.deferred?.reject(e);
throw e;
}
return this.ready();
}
Expand Down
13 changes: 1 addition & 12 deletions src/managers/WebviewViewProviderManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,7 @@ describe('WebviewViewProviderManager', () => {

before(() => {
context = {
...vscode.context,
extensionPath: '',
subscriptions: [],
asAbsolutePath: () => { },
globalState: {
get: () => {

},
update: () => {

}
}
...vscode.context
};

config.host = '86.75.30.9';
Expand Down
13 changes: 11 additions & 2 deletions src/mockVscode.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { EventEmitter } from 'eventemitter3';
import type { Command, Range, TreeDataProvider, TreeItemCollapsibleState, Uri, WorkspaceFolder, ConfigurationScope, ExtensionContext, WorkspaceConfiguration, OutputChannel, QuickPickItem } from 'vscode';
import * as path from 'path';

//copied from vscode to help with unit tests
enum QuickPickItemKind {
Expand All @@ -14,6 +15,7 @@ afterEach(() => {
});

export let vscode = {
version: '1.89.1',
env: {
//disable all telemetry reporting during unit tests
telemetryConfiguration: {
Expand All @@ -22,6 +24,11 @@ export let vscode = {
isCrashEnabled: false
}
},
ExtensionMode: {
Production: 1,
Development: 2,
Test: 3
},
CompletionItem: class { },
CodeLens: class { },
CodeAction: class { },
Expand Down Expand Up @@ -72,10 +79,12 @@ export let vscode = {

}
},
CodeActionKind: {
},
context: {
subscriptions: [],
asAbsolutePath: () => {
return '';
asAbsolutePath: (arg) => {
return path.resolve(arg);
},
extensionUri: undefined as Uri,
extensionPath: '',
Expand Down

0 comments on commit 3a92232

Please sign in to comment.