diff --git a/extensions/typescript-language-features/src/test/server.test.ts b/extensions/typescript-language-features/src/test/server.test.ts new file mode 100644 index 00000000000..bd22203d8d3 --- /dev/null +++ b/extensions/typescript-language-features/src/test/server.test.ts @@ -0,0 +1,76 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { + return new Promise((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); + }); +}); + diff --git a/extensions/typescript-language-features/src/tsServer/server.ts b/extensions/typescript-language-features/src/tsServer/server.ts index 1bfb290fa0a..10a001d52c2 100644 --- a/extensions/typescript-language-features/src/tsServer/server.ts +++ b/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; private readonly _requestQueue = new RequestQueue(); @@ -246,18 +310,25 @@ export class TypeScriptServer extends Disposable { private readonly _pendingResponses = new Set(); 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(this._childProcess.stdout!)); + this._reader = this._register(new Reader(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()); @@ -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; } diff --git a/extensions/typescript-language-features/src/typescriptServiceClient.ts b/extensions/typescript-language-features/src/typescriptServiceClient.ts index 8f8f45aaa34..b478e9ed080 100644 --- a/extensions/typescript-language-features/src/typescriptServiceClient.ts +++ b/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; diff --git a/extensions/typescript-language-features/src/utils/telemetry.ts b/extensions/typescript-language-features/src/utils/telemetry.ts index e3b5f09adc7..fe6ea8d292a 100644 --- a/extensions/typescript-language-features/src/utils/telemetry.ts +++ b/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) {