From 3a9223266faa0a3a7bc54bfccdda3fb8937465cd Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 4 Jun 2024 16:09:20 -0400 Subject: [PATCH] Fix lsp crash tracker (#576) * Fix bug with lsp crash tracker * Add unit tests, handle a few error flows --- src/LanguageServerManager.spec.ts | 70 ++++++++-- src/LanguageServerManager.ts | 122 ++++++++++-------- .../WebviewViewProviderManager.spec.ts | 13 +- src/mockVscode.spec.ts | 13 +- 4 files changed, 141 insertions(+), 77 deletions(-) diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 94ad9c7e..581cfbd6 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -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(); @@ -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']; @@ -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'); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index a3c2c435..9e264bff 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -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(); @@ -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(); } diff --git a/src/managers/WebviewViewProviderManager.spec.ts b/src/managers/WebviewViewProviderManager.spec.ts index 190ed022..fd073fad 100644 --- a/src/managers/WebviewViewProviderManager.spec.ts +++ b/src/managers/WebviewViewProviderManager.spec.ts @@ -19,18 +19,7 @@ describe('WebviewViewProviderManager', () => { before(() => { context = { - ...vscode.context, - extensionPath: '', - subscriptions: [], - asAbsolutePath: () => { }, - globalState: { - get: () => { - - }, - update: () => { - - } - } + ...vscode.context }; config.host = '86.75.30.9'; diff --git a/src/mockVscode.spec.ts b/src/mockVscode.spec.ts index 2b1d7aec..79f3f279 100644 --- a/src/mockVscode.spec.ts +++ b/src/mockVscode.spec.ts @@ -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 { @@ -14,6 +15,7 @@ afterEach(() => { }); export let vscode = { + version: '1.89.1', env: { //disable all telemetry reporting during unit tests telemetryConfiguration: { @@ -22,6 +24,11 @@ export let vscode = { isCrashEnabled: false } }, + ExtensionMode: { + Production: 1, + Development: 2, + Test: 3 + }, CompletionItem: class { }, CodeLens: class { }, CodeAction: class { }, @@ -72,10 +79,12 @@ export let vscode = { } }, + CodeActionKind: { + }, context: { subscriptions: [], - asAbsolutePath: () => { - return ''; + asAbsolutePath: (arg) => { + return path.resolve(arg); }, extensionUri: undefined as Uri, extensionPath: '',