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`)

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 */ }
 

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<Proto.Event>;
 	readonly onExit: vscode.Event<any>;
 	readonly onError: vscode.Event<any>;
-	readonly onReaderError: vscode.Event<Error>;
 
 	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<Proto.Response>;
 	private readonly _requestQueue = new RequestQueue();
 	private readonly _callbacks = new CallbackMap<Proto.Response>();
 	private readonly _pendingResponses = new Set<number>();
@@ -90,14 +87,17 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe
 		private readonly _tracer: Tracer,
 	) {
 		super();
-		this._reader = this._register(new Reader<Proto.Response>(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<any>());
 	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<any>());
 	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<any>());
 	public readonly onError = this._onError.event;
 
-	public get onReaderError() { return this.semanticServer.onReaderError; }
-
 	public get tsServerLogFile() { return this.semanticServer.tsServerLogFile; }
 
 	public kill(): void {

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();
-	}
-}

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)) {

extensions/typescript-language-features/src/utils/wireProtocol.ts → 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<T> extends Disposable {
+class Reader<T> extends Disposable {
 
 	private readonly buffer: ProtocolBuffer = new ProtocolBuffer();
 	private nextMessageLength: number = -1;
@@ -123,3 +126,36 @@ export class Reader<T> extends Disposable {
 		}
 	}
 }
+
+export class ChildServerProcess extends Disposable implements TsServerProcess {
+	private readonly _reader: Reader<Proto.Response>;
+
+	public constructor(
+		private readonly _process: ChildProcess,
+	) {
+		super();
+		this._reader = this._register(new Reader<Proto.Response>(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();
+	}
+}