From 00a0a77785cc5654cfeb25368a6c63254e373a90 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 16 Jul 2020 15:10:07 -0700 Subject: [PATCH] Cleaning up ts server logic Getting ready to have worker based TS servers instead of ones that use a process - Use standard error handler instead of having separate reader error - Move all process based server logic into own file (`serverProcess`) --- .../src/test/server.test.ts | 5 ++- .../src/tsServer/server.ts | 36 ++++++++---------- .../src/tsServer/spawner.ts | 28 +------------- .../src/typescriptServiceClient.ts | 1 - .../{wireProtocol.ts => serverProcess.ts} | 38 ++++++++++++++++++- 5 files changed, 57 insertions(+), 51 deletions(-) rename extensions/typescript-language-features/src/utils/{wireProtocol.ts => serverProcess.ts} (80%) diff --git a/extensions/typescript-language-features/src/test/server.test.ts b/extensions/typescript-language-features/src/test/server.test.ts index 6803701e678..d7189bdfb49 100644 --- a/extensions/typescript-language-features/src/test/server.test.ts +++ b/extensions/typescript-language-features/src/test/server.test.ts @@ -43,8 +43,9 @@ class FakeServerProcess implements TsServerProcess { }); } - - on(_name: any, _handler: any) { /* noop */ } + onData(_handler: any) { /* noop */ } + onError(_handler: any) { /* noop */ } + onExit(_handler: any) { /* noop */ } kill(): void { /* noop */ } diff --git a/extensions/typescript-language-features/src/tsServer/server.ts b/extensions/typescript-language-features/src/tsServer/server.ts index 0ac92ca7a42..55a14642a1f 100644 --- a/extensions/typescript-language-features/src/tsServer/server.ts +++ b/extensions/typescript-language-features/src/tsServer/server.ts @@ -4,19 +4,17 @@ *--------------------------------------------------------------------------------------------*/ import * as fs from 'fs'; -import * as stream from 'stream'; import * as vscode from 'vscode'; import type * as Proto from '../protocol'; +import { EventName } from '../protocol.const'; +import { CallbackMap } from '../tsServer/callbackMap'; +import { RequestItem, RequestQueue, RequestQueueingType } from '../tsServer/requestQueue'; +import { TypeScriptServerError } from '../tsServer/serverError'; import { ServerResponse, TypeScriptRequests } from '../typescriptService'; import { Disposable } from '../utils/dispose'; import { TelemetryReporter } from '../utils/telemetry'; import Tracer from '../utils/tracer'; import { TypeScriptVersion } from '../utils/versionProvider'; -import { Reader } from '../utils/wireProtocol'; -import { CallbackMap } from './callbackMap'; -import { RequestItem, RequestQueue, RequestQueueingType } from './requestQueue'; -import { TypeScriptServerError } from './serverError'; -import { EventName } from '../protocol.const'; export interface OngoingRequestCanceller { tryCancelOngoingRequest(seq: number): boolean; @@ -47,7 +45,6 @@ export interface ITypeScriptServer { readonly onEvent: vscode.Event; readonly onExit: vscode.Event; readonly onError: vscode.Event; - readonly onReaderError: vscode.Event; readonly tsServerLogFile: string | undefined; @@ -65,17 +62,17 @@ export interface TsServerDelegate { } export interface TsServerProcess { - readonly stdout: stream.Readable; write(serverRequest: Proto.Request): void; - on(name: 'exit', handler: (code: number | null) => void): void; - on(name: 'error', handler: (error: Error) => void): void; + onData(handler: (data: Proto.Response) => void): void; + onExit(handler: (code: number | null) => void): void; + onError(handler: (error: Error) => void): void; kill(): void; } + export class ProcessBasedTsServer extends Disposable implements ITypeScriptServer { - private readonly _reader: Reader; private readonly _requestQueue = new RequestQueue(); private readonly _callbacks = new CallbackMap(); private readonly _pendingResponses = new Set(); @@ -90,14 +87,17 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe private readonly _tracer: Tracer, ) { super(); - this._reader = this._register(new Reader(this._process.stdout!)); - this._reader.onData(msg => this.dispatchMessage(msg)); - this._process.on('exit', code => { + this._process.onData(msg => { + this.dispatchMessage(msg); + }); + + this._process.onExit(code => { this._onExit.fire(code); this._callbacks.destroy('server exited'); }); - this._process.on('error', error => { + + this._process.onError(error => { this._onError.fire(error); this._callbacks.destroy('server errored'); }); @@ -112,8 +112,6 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe private readonly _onError = this._register(new vscode.EventEmitter()); public readonly onError = this._onError.event; - public get onReaderError() { return this._reader.onError; } - public get tsServerLogFile() { return this._tsServerLogFile; } private write(serverRequest: Proto.Request) { @@ -439,8 +437,6 @@ export class GetErrRoutingTsServer extends Disposable implements ITypeScriptServ private readonly _onError = this._register(new vscode.EventEmitter()); public readonly onError = this._onError.event; - public get onReaderError() { return this.mainServer.onReaderError; } - public get tsServerLogFile() { return this.mainServer.tsServerLogFile; } public kill(): void { @@ -577,8 +573,6 @@ export class SyntaxRoutingTsServer extends Disposable implements ITypeScriptServ private readonly _onError = this._register(new vscode.EventEmitter()); public readonly onError = this._onError.event; - public get onReaderError() { return this.semanticServer.onReaderError; } - public get tsServerLogFile() { return this.semanticServer.tsServerLogFile; } public kill(): void { diff --git a/extensions/typescript-language-features/src/tsServer/spawner.ts b/extensions/typescript-language-features/src/tsServer/spawner.ts index 02f13e7e2c4..5bc505d0bf6 100644 --- a/extensions/typescript-language-features/src/tsServer/spawner.ts +++ b/extensions/typescript-language-features/src/tsServer/spawner.ts @@ -3,11 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as child_process from 'child_process'; import * as path from 'path'; -import * as stream from 'stream'; import * as vscode from 'vscode'; -import type * as Proto from '../protocol'; import { ClientCapabilities, ClientCapability } from '../typescriptService'; import API from '../utils/api'; import { SeparateSyntaxServerConfiguration, TsServerLogLevel, TypeScriptServiceConfiguration } from '../utils/configuration'; @@ -16,10 +13,11 @@ import LogDirectoryProvider from '../utils/logDirectoryProvider'; import Logger from '../utils/logger'; import { TypeScriptPluginPathsProvider } from '../utils/pluginPathsProvider'; import { PluginManager } from '../utils/plugins'; +import { ChildServerProcess } from '../utils/serverProcess'; import { TelemetryReporter } from '../utils/telemetry'; import Tracer from '../utils/tracer'; import { TypeScriptVersion, TypeScriptVersionProvider } from '../utils/versionProvider'; -import { GetErrRoutingTsServer, ITypeScriptServer, PipeRequestCanceller, ProcessBasedTsServer, SyntaxRoutingTsServer, TsServerDelegate, TsServerProcess } from './server'; +import { GetErrRoutingTsServer, ITypeScriptServer, PipeRequestCanceller, ProcessBasedTsServer, SyntaxRoutingTsServer, TsServerDelegate } from './server'; const enum ServerKind { Main = 'main', @@ -269,25 +267,3 @@ export class TypeScriptServerSpawner { } } -class ChildServerProcess implements TsServerProcess { - - public constructor( - private readonly _process: child_process.ChildProcess, - ) { } - - get stdout(): stream.Readable { return this._process.stdout!; } - - write(serverRequest: Proto.Request): void { - this._process.stdin!.write(JSON.stringify(serverRequest) + '\r\n', 'utf8'); - } - - on(name: 'exit', handler: (code: number | null) => void): void; - on(name: 'error', handler: (error: Error) => void): void; - on(name: any, handler: any) { - this._process.on(name, handler); - } - - kill(): void { - this._process.kill(); - } -} diff --git a/extensions/typescript-language-features/src/typescriptServiceClient.ts b/extensions/typescript-language-features/src/typescriptServiceClient.ts index cc2926e20fb..0e9dd740636 100644 --- a/extensions/typescript-language-features/src/typescriptServiceClient.ts +++ b/extensions/typescript-language-features/src/typescriptServiceClient.ts @@ -424,7 +424,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.isRestarting = false; }); - handle.onReaderError(error => this.error('ReaderError', error)); handle.onEvent(event => this.dispatchEvent(event)); if (apiVersion.gte(API.v300)) { diff --git a/extensions/typescript-language-features/src/utils/wireProtocol.ts b/extensions/typescript-language-features/src/utils/serverProcess.ts similarity index 80% rename from extensions/typescript-language-features/src/utils/wireProtocol.ts rename to extensions/typescript-language-features/src/utils/serverProcess.ts index 64450f14280..495731f34a1 100644 --- a/extensions/typescript-language-features/src/utils/wireProtocol.ts +++ b/extensions/typescript-language-features/src/utils/serverProcess.ts @@ -3,8 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { ChildProcess } from 'child_process'; import * as stream from 'stream'; import * as vscode from 'vscode'; +import type * as Proto from '../protocol'; +import { TsServerProcess } from '../tsServer/server'; import { Disposable } from './dispose'; const defaultSize: number = 8192; @@ -80,7 +83,7 @@ class ProtocolBuffer { } } -export class Reader extends Disposable { +class Reader extends Disposable { private readonly buffer: ProtocolBuffer = new ProtocolBuffer(); private nextMessageLength: number = -1; @@ -123,3 +126,36 @@ export class Reader extends Disposable { } } } + +export class ChildServerProcess extends Disposable implements TsServerProcess { + private readonly _reader: Reader; + + public constructor( + private readonly _process: ChildProcess, + ) { + super(); + this._reader = this._register(new Reader(this._process.stdout!)); + } + + write(serverRequest: Proto.Request): void { + this._process.stdin!.write(JSON.stringify(serverRequest) + '\r\n', 'utf8'); + } + + onData(handler: (data: Proto.Response) => void): void { + this._reader.onData(handler); + } + + onExit(handler: (code: number | null) => void): void { + this._process.on('exit', handler); + } + + onError(handler: (err: Error) => void): void { + this._process.on('error', handler); + this._reader.onError(handler); + } + + kill(): void { + this._process.kill(); + this._reader.dispose(); + } +}