From 6682006c8bbfe5a5fb3a55d4244e98f0597d030c Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 11 Jan 2019 11:35:51 -0800 Subject: [PATCH] Use more explicit state for TS server states Try to prevent the tracked server state from getting into weird invalid states and make the state more explicit --- .../src/typescriptServiceClient.ts | 140 ++++++++++-------- 1 file changed, 82 insertions(+), 58 deletions(-) diff --git a/extensions/typescript-language-features/src/typescriptServiceClient.ts b/extensions/typescript-language-features/src/typescriptServiceClient.ts index 9e02bbedc81..864d028f6fe 100644 --- a/extensions/typescript-language-features/src/typescriptServiceClient.ts +++ b/extensions/typescript-language-features/src/typescriptServiceClient.ts @@ -34,6 +34,42 @@ export interface TsDiagnostics { readonly diagnostics: Proto.Diagnostic[]; } +namespace ServerState { + export const enum Type { + None, + Running, + Errored + } + + export const None = new class { readonly type = Type.None; }; + + export class Running { + readonly type = Type.Running; + constructor( + public readonly server: TypeScriptServer, + + /** + * API version obtained from the version picker after checking the corresponding path exists. + */ + public readonly apiVersion: API, + + /** + * Version reported by currently-running tsserver. + */ + public tsserverVersion: string | undefined, + ) { } + } + + export class Errored { + readonly type = Type.Errored; + constructor( + public readonly error: Error, + ) { } + } + + export type State = typeof None | Running | Errored; +} + export default class TypeScriptServiceClient extends Disposable implements ITypeScriptServiceClient { private static readonly WALK_THROUGH_SNIPPET_SCHEME_COLON = `${fileSchemes.walkThroughSnippet}:`; @@ -49,23 +85,13 @@ export default class TypeScriptServiceClient extends Disposable implements IType public readonly logger: Logger = new Logger(); private readonly typescriptServerSpawner: TypeScriptServerSpawner; - private forkedTsServer: TypeScriptServer | null; - private lastError: Error | null; + private serverState: ServerState.State = ServerState.None; private lastStart: number; private numberRestarts: number; private isRestarting: boolean = false; private loadingIndicator = new ServerInitializingIndicator(); public readonly telemetryReporter: TelemetryReporter; - /** - * API version obtained from the version picker after checking the corresponding path exists. - */ - private _apiVersion: API; - - /** - * Version reported by currently-running tsserver. - */ - private _tsserverVersion: string | undefined; public readonly bufferSyncSupport: BufferSyncSupport; public readonly diagnosticsManager: DiagnosticsManager; @@ -87,8 +113,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType }); this._onReady!.promise = p; - this.forkedTsServer = null; - this.lastError = null; this.numberRestarts = 0; this._configuration = TypeScriptServiceConfiguration.loadFromWorkspace(); @@ -96,8 +120,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.pluginPathsProvider = new TypeScriptPluginPathsProvider(this._configuration); this.versionPicker = new TypeScriptVersionPicker(this.versionProvider, this.workspaceState); - this._apiVersion = API.defaultVersion; - this._tsserverVersion = undefined; this.tracer = new Tracer(this.logger); this.bufferSyncSupport = new BufferSyncSupport(this, allModeIds); @@ -116,7 +138,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.pluginPathsProvider.updateConfiguration(this._configuration); this.tracer.updateConfiguration(); - if (this.forkedTsServer) { + if (this.serverState.type === ServerState.Type.Running) { if (this._configuration.checkJs !== oldConfiguration.checkJs || this._configuration.experimentalDecorators !== oldConfiguration.experimentalDecorators ) { @@ -129,7 +151,14 @@ export default class TypeScriptServiceClient extends Disposable implements IType } }, this, this._disposables); - this.telemetryReporter = this._register(new TelemetryReporter(() => this._tsserverVersion || this._apiVersion.versionString)); + this.telemetryReporter = this._register(new TelemetryReporter(() => { + if (this.serverState.type === ServerState.Type.Running) { + if (this.serverState.tsserverVersion) { + return this.serverState.tsserverVersion; + } + } + return this.apiVersion.versionString; + })); this.typescriptServerSpawner = new TypeScriptServerSpawner(this.versionProvider, this.logDirectoryProvider, this.pluginPathsProvider, this.logger, this.telemetryReporter, this.tracer); @@ -147,22 +176,21 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.bufferSyncSupport.dispose(); - if (this.forkedTsServer) { - this.forkedTsServer.kill(); + if (this.serverState.type === ServerState.Type.Running) { + this.serverState.server.kill(); } this.loadingIndicator.reset(); } public restartTsServer(): void { - if (this.forkedTsServer) { + if (this.serverState.type === ServerState.Type.Running) { this.info('Killing TS Server'); this.isRestarting = true; - this.forkedTsServer.kill(); - this.resetClientVersion(); + this.serverState.server.kill(); } - this.forkedTsServer = this.startService(true); + this.serverState = this.startService(true); } private readonly _onTsServerStarted = this._register(new vscode.EventEmitter()); @@ -193,7 +221,10 @@ export default class TypeScriptServiceClient extends Disposable implements IType public readonly onSurveyReady = this._onSurveyReady.event; public get apiVersion(): API { - return this._apiVersion; + if (this.serverState.type === ServerState.Type.Running) { + return this.serverState.apiVersion; + } + return API.defaultVersion; } public onReady(f: () => void): Promise { @@ -212,30 +243,30 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.telemetryReporter.logTelemetry(eventName, properties); } - private service(): TypeScriptServer | null { - if (this.forkedTsServer) { - return this.forkedTsServer; + private service(): TypeScriptServer { + if (this.serverState.type === ServerState.Type.Running) { + return this.serverState.server; } - if (this.lastError) { - throw this.lastError; + if (this.serverState.type === ServerState.Type.Errored) { + throw this.serverState.error; } - this.startService(); - if (this.forkedTsServer) { - return this.forkedTsServer; + const newState = this.startService(); + if (newState.type === ServerState.Type.Running) { + return newState.server; } throw new Error('Could not create TS service'); } public ensureServiceStarted() { - if (!this.forkedTsServer) { + if (this.serverState.type !== ServerState.Type.Running) { this.startService(); } } private token: number = 0; - private startService(resendModels: boolean = false): TypeScriptServer | null { + private startService(resendModels: boolean = false): ServerState.State { if (this.isDisposed) { - return null; + return ServerState.None; } let currentVersion = this.versionPicker.currentVersion; @@ -248,13 +279,13 @@ export default class TypeScriptServiceClient extends Disposable implements IType currentVersion = this.versionPicker.currentVersion; } - this._apiVersion = this.versionPicker.currentVersion.version || API.defaultVersion; + const apiVersion = this.versionPicker.currentVersion.version || API.defaultVersion; this.onDidChangeTypeScriptVersion(currentVersion); - this.lastError = null; let mytoken = ++this.token; const handle = this.typescriptServerSpawner.spawn(currentVersion, this.configuration, this.pluginManager); + this.serverState = new ServerState.Running(handle, apiVersion, undefined); this.lastStart = Date.now(); handle.onError((err: Error) => { @@ -267,7 +298,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType vscode.window.showErrorMessage(localize('serverExitedWithError', 'TypeScript language server exited with error. Error message is: {0}', err.message || err.name)); } - this.lastError = err; + this.serverState = new ServerState.Errored(err); this.error('TSServer errored with error.', err); if (handle.tsServerLogFile) { this.error(`TSServer log file: ${handle.tsServerLogFile}`); @@ -282,7 +313,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType */ this.logTelemetry('tsserver.error'); this.serviceExited(false); - this.resetClientVersion(); }); handle.onExit((code: any) => { @@ -317,16 +347,15 @@ export default class TypeScriptServiceClient extends Disposable implements IType handle.onEvent(event => this.dispatchEvent(event)); this._onReady!.resolve(); - this.forkedTsServer = handle; this._onTsServerStarted.fire(currentVersion.version); - if (this._apiVersion.gte(API.v300)) { + if (apiVersion.gte(API.v300)) { this.loadingIndicator.startedLoadingProject(undefined /* projectName */); } this.serviceStarted(resendModels); - return handle; + return this.serverState; } public onVersionStatusClicked(): Thenable { @@ -372,7 +401,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType return false; } - if (!this.forkedTsServer || !this.forkedTsServer.tsServerLogFile) { + if (this.serverState.type !== ServerState.Type.Running || !this.serverState.server.tsServerLogFile) { vscode.window.showWarningMessage(localize( 'typescript.openTsServerLog.noLogFile', 'TS Server has not started logging.')); @@ -380,7 +409,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType } try { - await vscode.commands.executeCommand('revealFileInOS', vscode.Uri.file(this.forkedTsServer.tsServerLogFile)); + await vscode.commands.executeCommand('revealFileInOS', vscode.Uri.file(this.serverState.server.tsServerLogFile)); return true; } catch { vscode.window.showWarningMessage(localize( @@ -437,10 +466,8 @@ export default class TypeScriptServiceClient extends Disposable implements IType id: MessageAction; } - this.forkedTsServer = null; - if (!restart) { - this.resetClientVersion(); - } else { + this.serverState = ServerState.None; + if (restart) { const diff = Date.now() - this.lastStart; this.numberRestarts++; let startService = true; @@ -464,7 +491,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType } */ this.logTelemetry('serviceExited'); - this.resetClientVersion(); } else if (diff < 60 * 1000 /* 1 Minutes */) { this.lastStart = Date.now(); prompt = vscode.window.showWarningMessage( @@ -490,7 +516,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType } public normalizedPath(resource: vscode.Uri): string | undefined { - if (this._apiVersion.gte(API.v213)) { + if (this.apiVersion.gte(API.v213)) { if (resource.scheme === fileSchemes.walkThroughSnippet || resource.scheme === fileSchemes.untitled) { const dirName = path.dirname(resource.path); const fileName = this.inMemoryResourcePrefix + path.basename(resource.path); @@ -524,11 +550,11 @@ export default class TypeScriptServiceClient extends Disposable implements IType } private get inMemoryResourcePrefix(): string { - return this._apiVersion.gte(API.v270) ? '^' : ''; + return this.apiVersion.gte(API.v270) ? '^' : ''; } public toResource(filepath: string): vscode.Uri { - if (this._apiVersion.gte(API.v213)) { + if (this.apiVersion.gte(API.v213)) { if (filepath.startsWith(TypeScriptServiceClient.WALK_THROUGH_SNIPPET_SCHEME_COLON) || (filepath.startsWith(fileSchemes.untitled + ':')) ) { let resource = vscode.Uri.parse(filepath); @@ -694,7 +720,9 @@ export default class TypeScriptServiceClient extends Disposable implements IType break; } if (telemetryData.telemetryEventName === 'projectInfo') { - this._tsserverVersion = properties['version']; + if (this.serverState.type === ServerState.Type.Running) { + this.serverState = new ServerState.Running(this.serverState.server, this.serverState.apiVersion, properties['version']); + } } /* __GDPR__ @@ -711,13 +739,9 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.logTelemetry(telemetryData.telemetryEventName, properties); } - private resetClientVersion() { - this._apiVersion = API.defaultVersion; - this._tsserverVersion = undefined; - } private configurePlugin(pluginName: string, configuration: {}): any { - if (this._apiVersion.gte(API.v314)) { + if (this.apiVersion.gte(API.v314)) { this.executeWithoutWaitingForResponse('configurePlugin', { pluginName, configuration }); } }