Working on getting ts server class in a testable state

extensions/typescript-language-features/src/test/server.test.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import 'mocha';
+import * as stream from 'stream';
+import { PipeRequestCanceller, ServerProcess, TypeScriptServer } from '../tsServer/server';
+import { nulToken } from '../utils/cancellation';
+import Logger from '../utils/logger';
+import TelemetryReporter from '../utils/telemetry';
+import Tracer from '../utils/tracer';
+import * as Proto from '../protocol';
+
+
+const NoopTelemetryReporter = new class implements TelemetryReporter {
+	logTelemetry(): void { /* noop */ }
+	dispose(): void { /* noop */ }
+};
+
+class FakeServerProcess implements ServerProcess {
+	private readonly _out: stream.PassThrough;
+
+	private readonly writeListeners = new Set<(data: Buffer) => void>();
+	public stdout: stream.PassThrough;
+
+	constructor() {
+		this._out = new stream.PassThrough();
+		this.stdout = this._out;
+	}
+
+	public write(data: Proto.Request) {
+		const listeners = Array.from(this.writeListeners);
+		this.writeListeners.clear();
+
+		setImmediate(() => {
+			for (const listener of listeners) {
+				listener(Buffer.from(JSON.stringify(data), 'utf8'));
+			}
+			const body = Buffer.from(JSON.stringify({ 'seq': data.seq, 'type': 'response', 'command': data.command, 'request_seq': data.seq, 'success': true }), 'utf8');
+			this._out.write(Buffer.from(`Content-Length: ${body.length}\r\n\r\n${body}`, 'utf8'));
+		});
+	}
+
+
+	on(_name: any, _handler: any) { /* noop */ }
+
+	kill(): void { /* noop */ }
+
+	public onWrite(): Promise<any> {
+		return new Promise<string>((resolve) => {
+			this.writeListeners.add((data) => {
+				resolve(JSON.parse(data.toString()));
+			});
+		});
+	}
+}
+
+suite('Server', () => {
+	const tracer = new Tracer(new Logger());
+
+	test('should send requests with increasing sequence numbers', async () => {
+		const process = new FakeServerProcess();
+		const server = new TypeScriptServer(process, undefined, new PipeRequestCanceller(undefined, tracer), undefined!, NoopTelemetryReporter, tracer);
+
+		const onWrite1 = process.onWrite();
+		server.executeImpl('geterr', {}, { isAsync: false, token: nulToken, expectsResult: true });
+		assert.strictEqual((await onWrite1).seq, 0);
+
+		const onWrite2 = process.onWrite();
+		server.executeImpl('geterr', {}, { isAsync: false, token: nulToken, expectsResult: true });
+		assert.strictEqual((await onWrite2).seq, 1);
+	});
+});
+

extensions/typescript-language-features/src/tsServer/server.ts

@@ -3,9 +3,10 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import * as cp from 'child_process';
+import * as child_process from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
+import * as stream from 'stream';
 import * as vscode from 'vscode';
 import * as Proto from '../protocol';
 import { ServerResponse } from '../typescriptService';
@@ -125,7 +126,13 @@ export class TypeScriptServerSpawner {
 		const childProcess = electron.fork(version.tsServerPath, args, this.getForkOptions());
 		this._logger.info('Started TSServer');
 
-		return new TypeScriptServer(childProcess, tsServerLogFile, cancellationPipeName, version, this._telemetryReporter, this._tracer);
+		return new TypeScriptServer(
+			new ChildServerProcess(childProcess),
+			tsServerLogFile,
+			new PipeRequestCanceller(cancellationPipeName, this._tracer),
+			version,
+			this._telemetryReporter,
+			this._tracer);
 	}
 
 	private getForkOptions() {
@@ -239,6 +246,63 @@ export class TypeScriptServerSpawner {
 	}
 }
 
+export interface OngoingRequestCanceller {
+	tryCancelOngoingRequest(seq: number): boolean;
+}
+
+export class PipeRequestCanceller implements OngoingRequestCanceller {
+	public constructor(
+		private readonly _cancellationPipeName: string | undefined,
+		private readonly _tracer: Tracer,
+	) { }
+
+	public tryCancelOngoingRequest(seq: number): boolean {
+		if (!this._cancellationPipeName) {
+			return false;
+		}
+		this._tracer.logTrace(`TypeScript Server: trying to cancel ongoing request with sequence number ${seq}`);
+		try {
+			fs.writeFileSync(this._cancellationPipeName + seq, '');
+		} catch {
+			// noop
+		}
+		return true;
+	}
+}
+
+export interface ServerProcess {
+	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;
+
+	kill(): void;
+}
+
+class ChildServerProcess implements ServerProcess {
+
+	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();
+	}
+}
+
 export class TypeScriptServer extends Disposable {
 	private readonly _reader: Reader<Proto.Response>;
 	private readonly _requestQueue = new RequestQueue();
@@ -246,18 +310,25 @@ export class TypeScriptServer extends Disposable {
 	private readonly _pendingResponses = new Set<number>();
 
 	constructor(
-		private readonly _childProcess: cp.ChildProcess,
+		private readonly _process: ServerProcess,
 		private readonly _tsServerLogFile: string | undefined,
-		private readonly _cancellationPipeName: string | undefined,
+		private readonly _requestCanceller: OngoingRequestCanceller,
 		private readonly _version: TypeScriptVersion,
 		private readonly _telemetryReporter: TelemetryReporter,
 		private readonly _tracer: Tracer,
 	) {
 		super();
-		this._reader = this._register(new Reader<Proto.Response>(this._childProcess.stdout!));
+		this._reader = this._register(new Reader<Proto.Response>(this._process.stdout!));
 		this._reader.onData(msg => this.dispatchMessage(msg));
-		this._childProcess.on('exit', code => this.handleExit(code));
-		this._childProcess.on('error', error => this.handleError(error));
+
+		this._process.on('exit', code => {
+			this._onExit.fire(code);
+			this._callbacks.destroy('server exited');
+		});
+		this._process.on('error', error => {
+			this._onError.fire(error);
+			this._callbacks.destroy('server errored');
+		});
 	}
 
 	private readonly _onEvent = this._register(new vscode.EventEmitter<Proto.Event>());
@@ -273,8 +344,8 @@ export class TypeScriptServer extends Disposable {
 
 	public get tsServerLogFile() { return this._tsServerLogFile; }
 
-	public write(serverRequest: Proto.Request) {
-		this._childProcess.stdin!.write(JSON.stringify(serverRequest) + '\r\n', 'utf8');
+	private write(serverRequest: Proto.Request) {
+		this._process.write(serverRequest);
 	}
 
 	public dispose() {
@@ -284,17 +355,7 @@ export class TypeScriptServer extends Disposable {
 	}
 
 	public kill() {
-		this._childProcess.kill();
-	}
-
-	private handleExit(error: any) {
-		this._onExit.fire(error);
-		this._callbacks.destroy('server exited');
-	}
-
-	private handleError(error: any) {
-		this._onError.fire(error);
-		this._callbacks.destroy('server errored');
+		this._process.kill();
 	}
 
 	private dispatchMessage(message: Proto.Message) {
@@ -334,13 +395,7 @@ export class TypeScriptServer extends Disposable {
 				return true;
 			}
 
-			if (this._cancellationPipeName) {
-				this._tracer.logTrace(`TypeScript Server: trying to cancel ongoing request with sequence number ${seq}`);
-				try {
-					fs.writeFileSync(this._cancellationPipeName + seq, '');
-				} catch {
-					// noop
-				}
+			if (this._requestCanceller.tryCancelOngoingRequest(seq)) {
 				return true;
 			}
 

extensions/typescript-language-features/src/typescriptServiceClient.ts

@@ -20,7 +20,7 @@ import LogDirectoryProvider from './utils/logDirectoryProvider';
 import Logger from './utils/logger';
 import { TypeScriptPluginPathsProvider } from './utils/pluginPathsProvider';
 import { PluginManager } from './utils/plugins';
-import TelemetryReporter from './utils/telemetry';
+import TelemetryReporter, { VSCodeTelemetryReporter } from './utils/telemetry';
 import Tracer from './utils/tracer';
 import { inferredProjectConfig } from './utils/tsconfig';
 import { TypeScriptVersionPicker } from './utils/versionPicker';
@@ -152,7 +152,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType
 			}
 		}, this, this._disposables);
 
-		this.telemetryReporter = this._register(new TelemetryReporter(() => {
+		this.telemetryReporter = this._register(new VSCodeTelemetryReporter(() => {
 			if (this.serverState.type === ServerState.Type.Running) {
 				if (this.serverState.tsserverVersion) {
 					return this.serverState.tsserverVersion;

extensions/typescript-language-features/src/utils/telemetry.ts

@@ -13,15 +13,14 @@ interface PackageInfo {
 	readonly aiKey: string;
 }
 
-export default class TelemetryReporter {
-	private _reporter: VsCodeTelemetryReporter | null = null;
+export default interface TelemetryReporter {
+	logTelemetry(eventName: string, properties?: { [prop: string]: string }): void;
 
-	dispose() {
-		if (this._reporter) {
-			this._reporter.dispose();
-			this._reporter = null;
-		}
-	}
+	dispose(): void;
+}
+
+export class VSCodeTelemetryReporter implements TelemetryReporter {
+	private _reporter: VsCodeTelemetryReporter | null = null;
 
 	constructor(
 		private readonly clientVersionDelegate: () => string
@@ -45,6 +44,13 @@ export default class TelemetryReporter {
 		}
 	}
 
+	public dispose() {
+		if (this._reporter) {
+			this._reporter.dispose();
+			this._reporter = null;
+		}
+	}
+
 	@memoize
 	private get reporter(): VsCodeTelemetryReporter | null {
 		if (this.packageInfo && this.packageInfo.aiKey) {