From dbeeecbaedaf236ccb423685a17b95ee5476fc97 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Sep 2018 15:30:33 -0700 Subject: [PATCH] Refactoring to create TS Server object Rebase of a number of incremental changes listed below Move first level dispatchMessage into ForkedTsServerProcess Goal is to move callbacks and other per-server state into `ForkedTsServerProcess` Create forked ts server object syncrounously There is no reason for this to be async anymore. Making this object sync reduces complexity and makes the code easier to reason about Moving server relate functions into ForkedTSServer object The goal here is to have a single "server" object that keeps track of all its relevant state. The service client would manage one of these servers at a time, starting new ones if needed and dispatching to old ones Split server into own file Use switch case instead of conditionals Make pendingResponses readonly Add typings for callback item Improve naming - Use more descriptive names - Preview private vars with _ Use passed in version for getting command line args Attach webview click handler to window instead of to document body Fixes #48926 Change error handling for ts server exit and error - Don't fire twice on error (once for the `once` and once for the `onError`) - Flush callbacks on both exit and error. Remove cancellationPipeName as state Remove obsolete comment Move all env generation into generatePatchedEnv Extract server spawn into static method Move spawn from static to be own factory class Move providers from arguments to state on the spawner Update js/ts grammar Remove duplicate error handler Cleaning up server fork - Standarize names - Extract methods - Move some function to be private statics - Move logging out of electron and into server.ts Use undefined instead of null for optional value --- .../syntaxes/JavaScript.tmLanguage.json | 41 +- .../syntaxes/JavaScriptReact.tmLanguage.json | 41 +- .../syntaxes/TypeScript.tmLanguage.json | 41 +- .../syntaxes/TypeScriptReact.tmLanguage.json | 41 +- .../src/server.ts | 476 ++++++++++++++ .../src/typescriptServiceClient.ts | 614 +++--------------- .../src/utils/electron.ts | 23 +- .../src/utils/logDirectoryProvider.ts | 2 +- .../webview/electron-browser/webview-pre.js | 6 +- 9 files changed, 666 insertions(+), 619 deletions(-) create mode 100644 extensions/typescript-language-features/src/server.ts diff --git a/extensions/javascript/syntaxes/JavaScript.tmLanguage.json b/extensions/javascript/syntaxes/JavaScript.tmLanguage.json index 7ea9fe439fb..cbd8b7af2a5 100644 --- a/extensions/javascript/syntaxes/JavaScript.tmLanguage.json +++ b/extensions/javascript/syntaxes/JavaScript.tmLanguage.json @@ -4,7 +4,7 @@ "If you want to provide a fix or improvement, please create a pull request against the original repository.", "Once accepted there, we are happy to receive an update request." ], - "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/67d50d27f0c459e6ccc9adc6645454ecb1c2406c", + "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/c90418c57af09ddc7bd6827895052a6276ef8a9f", "name": "JavaScript (with React support)", "scopeName": "source.js", "patterns": [ @@ -30,9 +30,6 @@ { "include": "#string" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -209,9 +206,6 @@ { "include": "#regex" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -599,7 +593,7 @@ "include": "#comment" }, { - "begin": "(?x)(?=((\\b(?\\s*$)", + "begin": "^(///)\\s*(?=<(reference|amd-dependency|amd-module)(\\s+(path|types|no-default-lib|lib|name)\\s*=\\s*((\\'([^\\'\\\\]|\\\\\\'|\\\\)*\\')|(\\\"([^\\\"\\\\]|\\\\\\\"|\\\\)*\\\")|(\\`([^\\`\\\\]|\\\\\\`|\\\\)*\\`)))+\\s*/>\\s*$)", "beginCaptures": { "1": { "name": "punctuation.definition.comment.js" diff --git a/extensions/javascript/syntaxes/JavaScriptReact.tmLanguage.json b/extensions/javascript/syntaxes/JavaScriptReact.tmLanguage.json index 8cb16316df7..080a370cd36 100644 --- a/extensions/javascript/syntaxes/JavaScriptReact.tmLanguage.json +++ b/extensions/javascript/syntaxes/JavaScriptReact.tmLanguage.json @@ -4,7 +4,7 @@ "If you want to provide a fix or improvement, please create a pull request against the original repository.", "Once accepted there, we are happy to receive an update request." ], - "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/67d50d27f0c459e6ccc9adc6645454ecb1c2406c", + "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/c90418c57af09ddc7bd6827895052a6276ef8a9f", "name": "JavaScript (with React support)", "scopeName": "source.js.jsx", "patterns": [ @@ -30,9 +30,6 @@ { "include": "#string" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -209,9 +206,6 @@ { "include": "#regex" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -599,7 +593,7 @@ "include": "#comment" }, { - "begin": "(?x)(?=((\\b(?\\s*$)", + "begin": "^(///)\\s*(?=<(reference|amd-dependency|amd-module)(\\s+(path|types|no-default-lib|lib|name)\\s*=\\s*((\\'([^\\'\\\\]|\\\\\\'|\\\\)*\\')|(\\\"([^\\\"\\\\]|\\\\\\\"|\\\\)*\\\")|(\\`([^\\`\\\\]|\\\\\\`|\\\\)*\\`)))+\\s*/>\\s*$)", "beginCaptures": { "1": { "name": "punctuation.definition.comment.js.jsx" diff --git a/extensions/typescript-basics/syntaxes/TypeScript.tmLanguage.json b/extensions/typescript-basics/syntaxes/TypeScript.tmLanguage.json index 1003c443970..b49b1aaaea8 100644 --- a/extensions/typescript-basics/syntaxes/TypeScript.tmLanguage.json +++ b/extensions/typescript-basics/syntaxes/TypeScript.tmLanguage.json @@ -4,7 +4,7 @@ "If you want to provide a fix or improvement, please create a pull request against the original repository.", "Once accepted there, we are happy to receive an update request." ], - "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/3f8583969690a13fc8d41cbc788d4ab5a19248ca", + "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/c90418c57af09ddc7bd6827895052a6276ef8a9f", "name": "TypeScript", "scopeName": "source.ts", "patterns": [ @@ -30,9 +30,6 @@ { "include": "#string" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -206,9 +203,6 @@ { "include": "#regex" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -596,7 +590,7 @@ "include": "#comment" }, { - "begin": "(?x)(?=((\\b(?\\s*$)", + "begin": "^(///)\\s*(?=<(reference|amd-dependency|amd-module)(\\s+(path|types|no-default-lib|lib|name)\\s*=\\s*((\\'([^\\'\\\\]|\\\\\\'|\\\\)*\\')|(\\\"([^\\\"\\\\]|\\\\\\\"|\\\\)*\\\")|(\\`([^\\`\\\\]|\\\\\\`|\\\\)*\\`)))+\\s*/>\\s*$)", "beginCaptures": { "1": { "name": "punctuation.definition.comment.ts" diff --git a/extensions/typescript-basics/syntaxes/TypeScriptReact.tmLanguage.json b/extensions/typescript-basics/syntaxes/TypeScriptReact.tmLanguage.json index 2b7cc3076b6..551992d927e 100644 --- a/extensions/typescript-basics/syntaxes/TypeScriptReact.tmLanguage.json +++ b/extensions/typescript-basics/syntaxes/TypeScriptReact.tmLanguage.json @@ -4,7 +4,7 @@ "If you want to provide a fix or improvement, please create a pull request against the original repository.", "Once accepted there, we are happy to receive an update request." ], - "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/67d50d27f0c459e6ccc9adc6645454ecb1c2406c", + "version": "https://github.com/Microsoft/TypeScript-TmLanguage/commit/c90418c57af09ddc7bd6827895052a6276ef8a9f", "name": "TypeScriptReact", "scopeName": "source.tsx", "patterns": [ @@ -30,9 +30,6 @@ { "include": "#string" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -209,9 +206,6 @@ { "include": "#regex" }, - { - "include": "#template" - }, { "include": "#comment" }, @@ -599,7 +593,7 @@ "include": "#comment" }, { - "begin": "(?x)(?=((\\b(?\\s*$)", + "begin": "^(///)\\s*(?=<(reference|amd-dependency|amd-module)(\\s+(path|types|no-default-lib|lib|name)\\s*=\\s*((\\'([^\\'\\\\]|\\\\\\'|\\\\)*\\')|(\\\"([^\\\"\\\\]|\\\\\\\"|\\\\)*\\\")|(\\`([^\\`\\\\]|\\\\\\`|\\\\)*\\`)))+\\s*/>\\s*$)", "beginCaptures": { "1": { "name": "punctuation.definition.comment.tsx" diff --git a/extensions/typescript-language-features/src/server.ts b/extensions/typescript-language-features/src/server.ts new file mode 100644 index 00000000000..7c42b3f9ea4 --- /dev/null +++ b/extensions/typescript-language-features/src/server.ts @@ -0,0 +1,476 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as cp from 'child_process'; +import * as path from 'path'; +import * as fs from 'fs'; +import * as vscode from 'vscode'; +import * as Proto from './protocol'; +import { Disposable } from './utils/dispose'; +import * as electron from './utils/electron'; +import Logger from './utils/logger'; +import TelemetryReporter from './utils/telemetry'; +import Tracer from './utils/tracer'; +import { Reader } from './utils/wireProtocol'; +import { TypeScriptVersion, TypeScriptVersionProvider } from './utils/versionProvider'; +import API from './utils/api'; +import { TypeScriptServiceConfiguration, TsServerLogLevel } from './utils/configuration'; +import { TypeScriptServerPlugin } from './utils/plugins'; +import { TypeScriptPluginPathsProvider } from './utils/pluginPathsProvider'; +import LogDirectoryProvider from './utils/logDirectoryProvider'; + +interface CallbackItem { + readonly onSuccess: (value: R) => void; + readonly onError: (err: any) => void; + readonly startTime: number; +} + +class CallbackMap { + private readonly _callbacks = new Map>(); + private readonly _asyncCallbacks = new Map>(); + private _pendingResponseCount = 0; + + public get pendingResponseCount() { + return this._pendingResponseCount; + } + + public destroy(cause: Error): void { + for (const callback of this._callbacks.values()) { + callback.onError(cause); + } + for (const callback of this._asyncCallbacks.values()) { + callback.onError(cause); + } + this._callbacks.clear(); + this._pendingResponseCount = 0; + } + + public add(seq: number, callback: CallbackItem, isAsync: boolean) { + if (isAsync) { + this._asyncCallbacks.set(seq, callback); + } else { + this._callbacks.set(seq, callback); + ++this._pendingResponseCount; + } + } + + public fetch(seq: number): CallbackItem | undefined { + const callback = this._callbacks.get(seq) || this._asyncCallbacks.get(seq); + this.delete(seq); + return callback; + } + + private delete(seq: number) { + if (this._callbacks.delete(seq)) { + --this._pendingResponseCount; + } else { + this._asyncCallbacks.delete(seq); + } + } +} + +interface RequestItem { + readonly request: Proto.Request; + callbacks: CallbackItem | null; + readonly isAsync: boolean; +} + +class RequestQueue { + private queue: RequestItem[] = []; + private sequenceNumber: number = 0; + + public get length(): number { + return this.queue.length; + } + + public push(item: RequestItem): void { + this.queue.push(item); + } + + public shift(): RequestItem | undefined { + return this.queue.shift(); + } + + public tryCancelPendingRequest(seq: number): boolean { + for (let i = 0; i < this.queue.length; i++) { + if (this.queue[i].request.seq === seq) { + this.queue.splice(i, 1); + return true; + } + } + return false; + } + + public createRequest(command: string, args: any): Proto.Request { + return { + seq: this.sequenceNumber++, + type: 'request', + command: command, + arguments: args + }; + } +} + +export class TypeScriptServerSpawner { + public constructor( + private readonly _versionProvider: TypeScriptVersionProvider, + private readonly _logDirectoryProvider: LogDirectoryProvider, + private readonly _pluginPathsProvider: TypeScriptPluginPathsProvider, + private readonly _logger: Logger, + private readonly _telemetryReporter: TelemetryReporter, + private readonly _tracer: Tracer, + ) { } + + public spawn( + version: TypeScriptVersion, + configuration: TypeScriptServiceConfiguration, + plugins: ReadonlyArray + ): TypeScriptServer { + const apiVersion = version.version || API.defaultVersion; + + const { args, cancellationPipeName, tsServerLogFile } = this.getTsServerArgs(configuration, version, plugins); + + if (TypeScriptServerSpawner.isLoggingEnabled(apiVersion, configuration)) { + if (tsServerLogFile) { + this._logger.info(`TSServer log file: ${tsServerLogFile}`); + } else { + this._logger.error('Could not create TSServer log directory'); + } + } + + this._logger.info('Forking TSServer'); + const childProcess = electron.fork(version.tsServerPath, args, this.getForkOptions()); + this._logger.info('Started TSServer'); + + return new TypeScriptServer(childProcess, tsServerLogFile, cancellationPipeName, this._logger, this._telemetryReporter, this._tracer); + } + + private getForkOptions() { + const debugPort = TypeScriptServerSpawner.getDebugPort(); + const tsServerForkOptions: electron.IForkOptions = { + execArgv: debugPort ? [`--inspect=${debugPort}`] : [], + }; + return tsServerForkOptions; + } + + private getTsServerArgs( + configuration: TypeScriptServiceConfiguration, + currentVersion: TypeScriptVersion, + plugins: ReadonlyArray, + ): { args: string[], cancellationPipeName: string | undefined, tsServerLogFile: string | undefined } { + const args: string[] = []; + let cancellationPipeName: string | undefined = undefined; + let tsServerLogFile: string | undefined = undefined; + + const apiVersion = currentVersion.version || API.defaultVersion; + + if (apiVersion.gte(API.v206)) { + if (apiVersion.gte(API.v250)) { + args.push('--useInferredProjectPerProjectRoot'); + } else { + args.push('--useSingleInferredProject'); + } + + if (configuration.disableAutomaticTypeAcquisition) { + args.push('--disableAutomaticTypingAcquisition'); + } + } + + if (apiVersion.gte(API.v208)) { + args.push('--enableTelemetry'); + } + + if (apiVersion.gte(API.v222)) { + cancellationPipeName = electron.getTempFile('tscancellation'); + args.push('--cancellationPipeName', cancellationPipeName + '*'); + } + + if (TypeScriptServerSpawner.isLoggingEnabled(apiVersion, configuration)) { + const logDir = this._logDirectoryProvider.getNewLogDirectory(); + if (logDir) { + tsServerLogFile = path.join(logDir, `tsserver.log`); + args.push('--logVerbosity', TsServerLogLevel.toString(configuration.tsServerLogLevel)); + args.push('--logFile', tsServerLogFile); + } + } + + if (apiVersion.gte(API.v230)) { + const pluginPaths = this._pluginPathsProvider.getPluginPaths(); + + if (plugins.length) { + args.push('--globalPlugins', plugins.map(x => x.name).join(',')); + + if (currentVersion.path === this._versionProvider.defaultVersion.path) { + pluginPaths.push(...plugins.map(x => x.path)); + } + } + + if (pluginPaths.length !== 0) { + args.push('--pluginProbeLocations', pluginPaths.join(',')); + } + } + + if (apiVersion.gte(API.v234)) { + if (configuration.npmLocation) { + args.push('--npmLocation', `"${configuration.npmLocation}"`); + } + } + + if (apiVersion.gte(API.v260)) { + args.push('--locale', TypeScriptServerSpawner.getTsLocale(configuration)); + } + + if (apiVersion.gte(API.v291)) { + args.push('--noGetErrOnBackgroundUpdate'); + } + + return { args, cancellationPipeName, tsServerLogFile }; + } + + private static getDebugPort(): number | undefined { + const value = process.env['TSS_DEBUG']; + if (value) { + const port = parseInt(value); + if (!isNaN(port)) { + return port; + } + } + return undefined; + } + + private static isLoggingEnabled(apiVersion: API, configuration: TypeScriptServiceConfiguration) { + return apiVersion.gte(API.v222) && + configuration.tsServerLogLevel !== TsServerLogLevel.Off; + } + + private static getTsLocale(configuration: TypeScriptServiceConfiguration): string { + return configuration.locale + ? configuration.locale + : vscode.env.language; + } +} + +export class TypeScriptServer extends Disposable { + private readonly _reader: Reader; + private readonly _requestQueue = new RequestQueue(); + private readonly _callbacks = new CallbackMap(); + + constructor( + private readonly _childProcess: cp.ChildProcess, + private readonly _tsServerLogFile: string | undefined, + private readonly _cancellationPipeName: string | undefined, + private readonly _logger: Logger, + private readonly _telemetryReporter: TelemetryReporter, + private readonly _tracer: Tracer, + ) { + super(); + this._reader = new Reader(this._childProcess.stdout); + this._reader.onData(msg => this.dispatchMessage(msg)); + this._childProcess.on('exit', code => this.handleExit(code)); + this._childProcess.on('error', error => this.handleError(error)); + } + + private readonly _onEvent = this._register(new vscode.EventEmitter()); + public readonly onEvent = this._onEvent.event; + + private readonly _onExit = this._register(new vscode.EventEmitter()); + public readonly onExit = this._onExit.event; + + 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; } + + public write(serverRequest: Proto.Request) { + this._childProcess.stdin.write(JSON.stringify(serverRequest) + '\r\n', 'utf8'); + } + + public dispose() { + super.dispose(); + this._callbacks.destroy(new Error('server disposed')); + } + + public kill() { + this._childProcess.kill(); + } + + private handleExit(error: any) { + this._onExit.fire(error); + this._callbacks.destroy(new Error('server exited')); + } + + private handleError(error: any) { + this._onError.fire(error); + this._callbacks.destroy(new Error('server errored')); + } + + private dispatchMessage(message: Proto.Message) { + try { + switch (message.type) { + case 'response': + this.dispatchResponse(message as Proto.Response); + break; + + case 'event': + const event = message as Proto.Event; + if (event.event === 'requestCompleted') { + const seq = (event as Proto.RequestCompletedEvent).body.request_seq; + const p = this._callbacks.fetch(seq); + if (p) { + this._tracer.traceRequestCompleted('requestCompleted', seq, p.startTime); + p.onSuccess(undefined); + } + } else { + this._onEvent.fire(event); + } + break; + + default: + throw new Error(`Unknown message type ${message.type} received`); + } + } finally { + this.sendNextRequests(); + } + } + + private tryCancelRequest(seq: number): boolean { + try { + if (this._requestQueue.tryCancelPendingRequest(seq)) { + this._tracer.logTrace(`TypeScript Server: canceled request with sequence number ${seq}`); + 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 + } + return true; + } + + this._tracer.logTrace(`TypeScript Server: tried to cancel request with sequence number ${seq}. But request got already delivered.`); + return false; + } finally { + const p = this._callbacks.fetch(seq); + if (p) { + p.onError(new Error(`Cancelled Request ${seq}`)); + } + } + } + + private dispatchResponse(response: Proto.Response) { + const callback = this._callbacks.fetch(response.request_seq); + if (!callback) { + return; + } + + this._tracer.traceResponse(response, callback.startTime); + if (response.success) { + callback.onSuccess(response); + } else { + callback.onError(response); + } + } + + public executeImpl(command: string, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean }): Promise { + const request = this._requestQueue.createRequest(command, args); + const requestInfo: RequestItem = { + request: request, + callbacks: null, + isAsync: executeInfo.isAsync + }; + let result: Promise; + if (executeInfo.expectsResult) { + let wasCancelled = false; + result = new Promise((resolve, reject) => { + requestInfo.callbacks = { onSuccess: resolve, onError: reject, startTime: Date.now() }; + if (executeInfo.token) { + executeInfo.token.onCancellationRequested(() => { + wasCancelled = true; + this.tryCancelRequest(request.seq); + }); + } + }).catch((err: any) => { + if (!wasCancelled) { + this._logger.error(`'${command}' request failed with error.`, err); + const properties = this.parseErrorText(err && err.message, command); + /* __GDPR__ + "languageServiceErrorResponse" : { + "command" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }, + "message" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, + "stack" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, + "errortext" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, + "${include}": [ + "${TypeScriptCommonProperties}" + ] + } + */ + this._telemetryReporter.logTelemetry('languageServiceErrorResponse', properties); + } + throw err; + }); + } else { + result = Promise.resolve(null); + } + this._requestQueue.push(requestInfo); + this.sendNextRequests(); + + return result; + } + + /** + * Given a `errorText` from a tsserver request indicating failure in handling a request, + * prepares a payload for telemetry-logging. + */ + private parseErrorText(errorText: string | undefined, command: string) { + const properties: ObjectMap = Object.create(null); + properties['command'] = command; + if (errorText) { + properties['errorText'] = errorText; + + const errorPrefix = 'Error processing request. '; + if (errorText.startsWith(errorPrefix)) { + const prefixFreeErrorText = errorText.substr(errorPrefix.length); + const newlineIndex = prefixFreeErrorText.indexOf('\n'); + if (newlineIndex >= 0) { + // Newline expected between message and stack. + properties['message'] = prefixFreeErrorText.substring(0, newlineIndex); + properties['stack'] = prefixFreeErrorText.substring(newlineIndex + 1); + } + } + } + return properties; + } + + private sendNextRequests(): void { + while (this._callbacks.pendingResponseCount === 0 && this._requestQueue.length > 0) { + const item = this._requestQueue.shift(); + if (item) { + this.sendRequest(item); + } + } + } + + private sendRequest(requestItem: RequestItem): void { + const serverRequest = requestItem.request; + this._tracer.traceRequest(serverRequest, !!requestItem.callbacks, this._requestQueue.length); + if (requestItem.callbacks) { + this._callbacks.add(serverRequest.seq, requestItem.callbacks, requestItem.isAsync); + } + try { + this.write(serverRequest); + } catch (err) { + const callback = this._callbacks.fetch(serverRequest.seq); + if (callback) { + callback.onError(err); + } + } + } +} + diff --git a/extensions/typescript-language-features/src/typescriptServiceClient.ts b/extensions/typescript-language-features/src/typescriptServiceClient.ts index 04bee73f6de..b93e5d11c95 100644 --- a/extensions/typescript-language-features/src/typescriptServiceClient.ts +++ b/extensions/typescript-language-features/src/typescriptServiceClient.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as cp from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as vscode from 'vscode'; @@ -11,11 +10,11 @@ import * as nls from 'vscode-nls'; import BufferSyncSupport from './features/bufferSyncSupport'; import { DiagnosticKind, DiagnosticsManager } from './features/diagnostics'; import * as Proto from './protocol'; +import { TypeScriptServer, TypeScriptServerSpawner } from './server'; import { ITypeScriptServiceClient } from './typescriptService'; import API from './utils/api'; import { TsServerLogLevel, TypeScriptServiceConfiguration } from './utils/configuration'; import { Disposable } from './utils/dispose'; -import * as electron from './utils/electron'; import * as fileSchemes from './utils/fileSchemes'; import * as is from './utils/is'; import LogDirectoryProvider from './utils/logDirectoryProvider'; @@ -27,130 +26,9 @@ import Tracer from './utils/tracer'; import { inferredProjectConfig } from './utils/tsconfig'; import { TypeScriptVersionPicker } from './utils/versionPicker'; import { TypeScriptVersion, TypeScriptVersionProvider } from './utils/versionProvider'; -import { Reader } from './utils/wireProtocol'; - const localize = nls.loadMessageBundle(); -interface CallbackItem { - readonly c: (value: any) => void; - readonly e: (err: any) => void; - readonly start: number; -} - -class CallbackMap { - private readonly callbacks: Map = new Map(); - private readonly asyncCallbacks: Map = new Map(); - public pendingResponses: number = 0; - - public destroy(e: any): void { - for (const callback of this.callbacks.values()) { - callback.e(e); - } - for (const callback of this.asyncCallbacks.values()) { - callback.e(e); - } - this.callbacks.clear(); - this.pendingResponses = 0; - } - - public add(seq: number, callback: CallbackItem, isAsync: boolean) { - if (isAsync) { - this.asyncCallbacks.set(seq, callback); - } else { - this.callbacks.set(seq, callback); - ++this.pendingResponses; - } - } - - public fetch(seq: number): CallbackItem | undefined { - const callback = this.callbacks.get(seq) || this.asyncCallbacks.get(seq); - this.delete(seq); - return callback; - } - - private delete(seq: number) { - if (this.callbacks.delete(seq)) { - --this.pendingResponses; - } else { - this.asyncCallbacks.delete(seq); - } - } -} - -interface RequestItem { - readonly request: Proto.Request; - callbacks: CallbackItem | null; - readonly isAsync: boolean; -} - -class RequestQueue { - private queue: RequestItem[] = []; - private sequenceNumber: number = 0; - - public get length(): number { - return this.queue.length; - } - - public push(item: RequestItem): void { - this.queue.push(item); - } - - public shift(): RequestItem | undefined { - return this.queue.shift(); - } - - public tryCancelPendingRequest(seq: number): boolean { - for (let i = 0; i < this.queue.length; i++) { - if (this.queue[i].request.seq === seq) { - this.queue.splice(i, 1); - return true; - } - } - return false; - } - - public createRequest(command: string, args: any): Proto.Request { - return { - seq: this.sequenceNumber++, - type: 'request', - command: command, - arguments: args - }; - } -} - -class ForkedTsServerProcess extends Disposable { - private readonly _reader: Reader; - - constructor( - private childProcess: cp.ChildProcess - ) { - super(); - this._reader = new Reader(this.childProcess.stdout); - } - - public onError(cb: (err: Error) => void): void { - this.childProcess.on('error', cb); - } - - public onExit(cb: (err: any) => void): void { - this.childProcess.on('exit', cb); - } - - public write(serverRequest: Proto.Request) { - this.childProcess.stdin.write(JSON.stringify(serverRequest) + '\r\n', 'utf8'); - } - - public get onReaderError() { return this._reader.onError; } - - public get onData() { return this._reader.onData; } - - public kill() { - this.childProcess.kill(); - } -} - export interface TsDiagnostics { readonly kind: DiagnosticKind; readonly resource: vscode.Uri; @@ -170,18 +48,14 @@ export default class TypeScriptServiceClient extends Disposable implements IType private tracer: Tracer; public readonly logger: Logger = new Logger(); - private tsServerLogFile: string | null = null; - private servicePromise: Thenable | null; + + private readonly typescriptServerSpawner: TypeScriptServerSpawner; + private forkedTsServer: TypeScriptServer | null; private lastError: Error | null; private lastStart: number; private numberRestarts: number; private isRestarting: boolean = false; - private cancellationPipeName: string | null = null; - - private requestQueue: RequestQueue; - private callbacks: CallbackMap; - public readonly telemetryReporter: TelemetryReporter; /** * API version obtained from the version picker after checking the corresponding path exists. @@ -211,12 +85,10 @@ export default class TypeScriptServiceClient extends Disposable implements IType }); this._onReady!.promise = p; - this.servicePromise = null; + this.forkedTsServer = null; this.lastError = null; this.numberRestarts = 0; - this.requestQueue = new RequestQueue(); - this.callbacks = new CallbackMap(); this._configuration = TypeScriptServiceConfiguration.loadFromWorkspace(); this.versionProvider = new TypeScriptVersionProvider(this._configuration); this.pluginPathsProvider = new TypeScriptPluginPathsProvider(this._configuration); @@ -242,7 +114,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.pluginPathsProvider.updateConfiguration(this._configuration); this.tracer.updateConfiguration(); - if (this.servicePromise) { + if (this.forkedTsServer) { if (this._configuration.checkJs !== oldConfiguration.checkJs || this._configuration.experimentalDecorators !== oldConfiguration.experimentalDecorators ) { @@ -254,8 +126,10 @@ export default class TypeScriptServiceClient extends Disposable implements IType } } }, this, this._disposables); - this.telemetryReporter = new TelemetryReporter(() => this._tsserverVersion || this._apiVersion.versionString); - this._register(this.telemetryReporter); + + this.telemetryReporter = this._register(new TelemetryReporter(() => this._tsserverVersion || this._apiVersion.versionString)); + + this.typescriptServerSpawner = new TypeScriptServerSpawner(this.versionProvider, this.logDirectoryProvider, this.pluginPathsProvider, this.logger, this.telemetryReporter, this.tracer); } public get configuration() { @@ -267,33 +141,20 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.bufferSyncSupport.dispose(); - if (this.servicePromise) { - this.servicePromise.then(childProcess => { - if (childProcess) { - childProcess.kill(); - } - }).then(undefined, () => void 0); + if (this.forkedTsServer) { + this.forkedTsServer.kill(); } } public restartTsServer(): void { - const start = () => { - this.servicePromise = this.startService(true); - return this.servicePromise; - }; - - if (this.servicePromise) { - this.servicePromise = this.servicePromise.then(childProcess => { - this.info('Killing TS Server'); - this.isRestarting = true; - if (childProcess) { - childProcess.kill(); - } - this.resetClientVersion(); - }).then(start); - } else { - start(); + if (this.forkedTsServer) { + this.info('Killing TS Server'); + this.isRestarting = true; + this.forkedTsServer.kill(); + this.resetClientVersion(); } + + this.forkedTsServer = this.startService(true); } private readonly _onTsServerStarted = this._register(new vscode.EventEmitter()); @@ -340,28 +201,28 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.telemetryReporter.logTelemetry(eventName, properties); } - private service(): Thenable { - if (this.servicePromise) { - return this.servicePromise; + private service(): TypeScriptServer | null { + if (this.forkedTsServer) { + return this.forkedTsServer; } if (this.lastError) { - return Promise.reject(this.lastError); + throw this.lastError; } this.startService(); - if (this.servicePromise) { - return this.servicePromise; + if (this.forkedTsServer) { + return this.forkedTsServer; } - return Promise.reject(new Error('Could not create TS service')); + throw new Error('Could not create TS service'); } public ensureServiceStarted() { - if (!this.servicePromise) { + if (!this.forkedTsServer) { this.startService(); } } private token: number = 0; - private startService(resendModels: boolean = false): Promise | null { + private startService(resendModels: boolean = false): TypeScriptServer | null { if (this.isDisposed) { return null; } @@ -379,98 +240,77 @@ export default class TypeScriptServiceClient extends Disposable implements IType this._apiVersion = this.versionPicker.currentVersion.version || API.defaultVersion; this.onDidChangeTypeScriptVersion(currentVersion); - this.requestQueue = new RequestQueue(); - this.callbacks = new CallbackMap(); this.lastError = null; let mytoken = ++this.token; - return this.servicePromise = new Promise(async (resolve, reject) => { - try { - const tsServerForkArgs = await this.getTsServerArgs(currentVersion); - const debugPort = this.getDebugPort(); - const tsServerForkOptions: electron.IForkOptions = { - execArgv: debugPort ? [`--inspect=${debugPort}`] : [] // [`--debug-brk=5859`] - }; - const childProcess = electron.fork(currentVersion.tsServerPath, tsServerForkArgs, tsServerForkOptions, this.logger); - childProcess.once('error', (err: Error) => { - this.lastError = err; - this.error('Starting TSServer failed with error.', err); - vscode.window.showErrorMessage(localize('serverCouldNotBeStarted', 'TypeScript language server couldn\'t be started. Error message is: {0}', err.message || err.name)); - /* __GDPR__ - "error" : { - "${include}": [ - "${TypeScriptCommonProperties}" - ] - } - */ - this.logTelemetry('error'); - this.resetClientVersion(); - return; - }); + const handle = this.typescriptServerSpawner.spawn(currentVersion, this.configuration, this.plugins); + this.lastStart = Date.now(); - this.info('Started TSServer'); - const handle = new ForkedTsServerProcess(childProcess); - this.lastStart = Date.now(); - - handle.onError((err: Error) => { - if (this.token !== mytoken) { - // this is coming from an old process - return; - } - this.lastError = err; - this.error('TSServer errored with error.', err); - if (this.tsServerLogFile) { - this.error(`TSServer log file: ${this.tsServerLogFile}`); - } - /* __GDPR__ - "tsserver.error" : { - "${include}": [ - "${TypeScriptCommonProperties}" - ] - } - */ - this.logTelemetry('tsserver.error'); - this.serviceExited(false); - }); - handle.onExit((code: any) => { - if (this.token !== mytoken) { - // this is coming from an old process - return; - } - if (code === null || typeof code === 'undefined') { - this.info('TSServer exited'); - } else { - this.error(`TSServer exited with code: ${code}`); - /* __GDPR__ - "tsserver.exitWithCode" : { - "code" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, - "${include}": [ - "${TypeScriptCommonProperties}" - ] - } - */ - this.logTelemetry('tsserver.exitWithCode', { code: code }); - } - - if (this.tsServerLogFile) { - this.info(`TSServer log file: ${this.tsServerLogFile}`); - } - this.serviceExited(!this.isRestarting); - this.isRestarting = false; - }); - - handle.onData(msg => this.dispatchMessage(msg)); - handle.onReaderError(error => this.error('ReaderError', error)); - - this._onReady!.resolve(); - resolve(handle); - this._onTsServerStarted.fire(currentVersion.version); - - this.serviceStarted(resendModels); - } catch (error) { - reject(error); + handle.onError((err: Error) => { + if (this.token !== mytoken) { + // this is coming from an old process + return; } + + if (err) { + vscode.window.showErrorMessage(localize('serverExitedWithError', 'TypeScript language server exited with error. Error message is: {0}', err.message || err.name)); + } + + this.lastError = err; + this.error('TSServer errored with error.', err); + if (handle.tsServerLogFile) { + this.error(`TSServer log file: ${handle.tsServerLogFile}`); + } + + /* __GDPR__ + "tsserver.error" : { + "${include}": [ + "${TypeScriptCommonProperties}" + ] + } + */ + this.logTelemetry('tsserver.error'); + this.serviceExited(false); + this.resetClientVersion(); }); + + handle.onExit((code: any) => { + if (this.token !== mytoken) { + // this is coming from an old process + return; + } + + if (code === null || typeof code === 'undefined') { + this.info('TSServer exited'); + } else { + this.error(`TSServer exited with code: ${code}`); + /* __GDPR__ + "tsserver.exitWithCode" : { + "code" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, + "${include}": [ + "${TypeScriptCommonProperties}" + ] + } + */ + this.logTelemetry('tsserver.exitWithCode', { code: code }); + } + + if (handle.tsServerLogFile) { + this.info(`TSServer log file: ${handle.tsServerLogFile}`); + } + this.serviceExited(!this.isRestarting); + this.isRestarting = false; + }); + + handle.onReaderError(error => this.error('ReaderError', error)); + handle.onEvent(event => this.dispatchEvent(event)); + + this._onReady!.resolve(); + this.forkedTsServer = handle; + this._onTsServerStarted.fire(currentVersion.version); + + this.serviceStarted(resendModels); + return handle; } public onVersionStatusClicked(): Thenable { @@ -516,7 +356,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType return false; } - if (!this.tsServerLogFile) { + if (!this.forkedTsServer || !this.forkedTsServer.tsServerLogFile) { vscode.window.showWarningMessage(localize( 'typescript.openTsServerLog.noLogFile', 'TS Server has not started logging.')); @@ -524,7 +364,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType } try { - await vscode.commands.executeCommand('revealFileInOS', vscode.Uri.parse(this.tsServerLogFile)); + await vscode.commands.executeCommand('revealFileInOS', vscode.Uri.parse(this.forkedTsServer.tsServerLogFile)); return true; } catch { vscode.window.showWarningMessage(localize( @@ -574,14 +414,10 @@ export default class TypeScriptServiceClient extends Disposable implements IType id: MessageAction; } - this.servicePromise = null; - this.tsServerLogFile = null; - this.callbacks.destroy(new Error('Service died.')); - this.callbacks = new CallbackMap(); + this.forkedTsServer = null; if (!restart) { this.resetClientVersion(); - } - else { + } else { const diff = Date.now() - this.lastStart; this.numberRestarts++; let startService = true; @@ -721,171 +557,19 @@ export default class TypeScriptServiceClient extends Disposable implements IType } private executeImpl(command: string, args: any, executeInfo: { isAsync: boolean, token?: vscode.CancellationToken, expectsResult: boolean }): Promise { - const request = this.requestQueue.createRequest(command, args); - const requestInfo: RequestItem = { - request: request, - callbacks: null, - isAsync: executeInfo.isAsync - }; - let result: Promise; - if (executeInfo.expectsResult) { - let wasCancelled = false; - result = new Promise((resolve, reject) => { - requestInfo.callbacks = { c: resolve, e: reject, start: Date.now() }; - if (executeInfo.token) { - executeInfo.token.onCancellationRequested(() => { - wasCancelled = true; - this.tryCancelRequest(request.seq); - }); - } - }).catch((err: any) => { - if (!wasCancelled) { - this.error(`'${command}' request failed with error.`, err); - const properties = this.parseErrorText(err && err.message, command); - /* __GDPR__ - "languageServiceErrorResponse" : { - "command" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }, - "message" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, - "stack" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, - "errortext" : { "classification": "CallstackOrException", "purpose": "PerformanceAndHealth" }, - "${include}": [ - "${TypeScriptCommonProperties}" - ] - } - */ - this.logTelemetry('languageServiceErrorResponse', properties); - } - throw err; - }); - } else { - result = Promise.resolve(null); + const server = this.service(); + if (!server) { + return Promise.reject(new Error('Could not load TS Server')); } - this.requestQueue.push(requestInfo); - this.sendNextRequests(); - - return result; + return server.executeImpl(command, args, executeInfo); } public interuptGetErr(f: () => R): R { return this.bufferSyncSupport.interuptGetErr(f); } - /** - * Given a `errorText` from a tsserver request indicating failure in handling a request, - * prepares a payload for telemetry-logging. - */ - private parseErrorText(errorText: string | undefined, command: string) { - const properties: ObjectMap = Object.create(null); - properties['command'] = command; - if (errorText) { - properties['errorText'] = errorText; - - const errorPrefix = 'Error processing request. '; - if (errorText.startsWith(errorPrefix)) { - const prefixFreeErrorText = errorText.substr(errorPrefix.length); - const newlineIndex = prefixFreeErrorText.indexOf('\n'); - if (newlineIndex >= 0) { - // Newline expected between message and stack. - properties['message'] = prefixFreeErrorText.substring(0, newlineIndex); - properties['stack'] = prefixFreeErrorText.substring(newlineIndex + 1); - } - } - } - return properties; - } - - private sendNextRequests(): void { - while (this.callbacks.pendingResponses === 0 && this.requestQueue.length > 0) { - const item = this.requestQueue.shift(); - if (item) { - this.sendRequest(item); - } - } - } - - private sendRequest(requestItem: RequestItem): void { - const serverRequest = requestItem.request; - this.tracer.traceRequest(serverRequest, !!requestItem.callbacks, this.requestQueue.length); - if (requestItem.callbacks) { - this.callbacks.add(serverRequest.seq, requestItem.callbacks, requestItem.isAsync); - } - this.service() - .then(childProcess => { - if (childProcess) { - childProcess.write(serverRequest); - } - }) - .then(undefined, err => { - const callback = this.callbacks.fetch(serverRequest.seq); - if (callback) { - callback.e(err); - } - }); - } - - private tryCancelRequest(seq: number): boolean { - try { - if (this.requestQueue.tryCancelPendingRequest(seq)) { - this.tracer.logTrace(`TypeScript Service: canceled request with sequence number ${seq}`); - return true; - } - - if (this.apiVersion.gte(API.v222) && this.cancellationPipeName) { - this.tracer.logTrace(`TypeScript Service: trying to cancel ongoing request with sequence number ${seq}`); - try { - fs.writeFileSync(this.cancellationPipeName + seq, ''); - } catch { - // noop - } - return true; - } - - this.tracer.logTrace(`TypeScript Service: tried to cancel request with sequence number ${seq}. But request got already delivered.`); - return false; - } finally { - const p = this.callbacks.fetch(seq); - if (p) { - p.e(new Error(`Cancelled Request ${seq}`)); - } - } - } - - private dispatchMessage(message: Proto.Message): void { - try { - if (message.type === 'response') { - const response: Proto.Response = message as Proto.Response; - const p = this.callbacks.fetch(response.request_seq); - if (p) { - this.tracer.traceResponse(response, p.start); - if (response.success) { - p.c(response); - } else { - p.e(response); - } - } - } else if (message.type === 'event') { - const event: Proto.Event = message; - this.tracer.traceEvent(event); - this.dispatchEvent(event); - } else { - throw new Error('Unknown message type ' + message.type + ' received'); - } - } finally { - this.sendNextRequests(); - } - } - private dispatchEvent(event: Proto.Event) { switch (event.event) { - case 'requestCompleted': - const seq = (event as Proto.RequestCompletedEvent).body.request_seq; - const p = this.callbacks.fetch(seq); - if (p) { - this.tracer.traceRequestCompleted('requestCompleted', seq, p.start); - p.c(undefined); - } - break; - case 'syntaxDiag': case 'semanticDiag': case 'suggestionDiag': @@ -990,97 +674,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType this.logTelemetry(telemetryData.telemetryEventName, properties); } - private async getTsServerArgs( - currentVersion: TypeScriptVersion - ): Promise { - const args: string[] = []; - - if (this.apiVersion.gte(API.v206)) { - if (this.apiVersion.gte(API.v250)) { - args.push('--useInferredProjectPerProjectRoot'); - } else { - args.push('--useSingleInferredProject'); - } - - if (this._configuration.disableAutomaticTypeAcquisition) { - args.push('--disableAutomaticTypingAcquisition'); - } - } - - if (this.apiVersion.gte(API.v208)) { - args.push('--enableTelemetry'); - } - - if (this.apiVersion.gte(API.v222)) { - this.cancellationPipeName = electron.getTempFile('tscancellation'); - args.push('--cancellationPipeName', this.cancellationPipeName + '*'); - } - - if (this.apiVersion.gte(API.v222)) { - if (this._configuration.tsServerLogLevel !== TsServerLogLevel.Off) { - const logDir = await this.logDirectoryProvider.getNewLogDirectory(); - if (logDir) { - this.tsServerLogFile = path.join(logDir, `tsserver.log`); - this.info(`TSServer log file: ${this.tsServerLogFile}`); - } else { - this.tsServerLogFile = null; - this.error('Could not create TSServer log directory'); - } - - if (this.tsServerLogFile) { - args.push('--logVerbosity', TsServerLogLevel.toString(this._configuration.tsServerLogLevel)); - args.push('--logFile', this.tsServerLogFile); - } - } - } - - if (this.apiVersion.gte(API.v230)) { - const pluginPaths = this.pluginPathsProvider.getPluginPaths(); - - if (this.plugins.length) { - args.push('--globalPlugins', this.plugins.map(x => x.name).join(',')); - - if (currentVersion.path === this.versionProvider.defaultVersion.path) { - pluginPaths.push(...this.plugins.map(x => x.path)); - } - } - - if (pluginPaths.length !== 0) { - args.push('--pluginProbeLocations', pluginPaths.join(',')); - } - } - - if (this.apiVersion.gte(API.v234)) { - if (this._configuration.npmLocation) { - args.push('--npmLocation', `"${this._configuration.npmLocation}"`); - } - } - - if (this.apiVersion.gte(API.v260)) { - const tsLocale = getTsLocale(this._configuration); - if (tsLocale) { - args.push('--locale', tsLocale); - } - } - - if (this.apiVersion.gte(API.v291)) { - args.push('--noGetErrOnBackgroundUpdate'); - } - - return args; - } - - private getDebugPort(): number | undefined { - const value = process.env['TSS_DEBUG']; - if (value) { - const port = parseInt(value); - if (!isNaN(port)) { - return port; - } - } - return undefined; - } - private resetClientVersion() { this._apiVersion = API.defaultVersion; this._tsserverVersion = undefined; @@ -1088,11 +681,6 @@ export default class TypeScriptServiceClient extends Disposable implements IType } -const getTsLocale = (configuration: TypeScriptServiceConfiguration): string | undefined => - (configuration.locale - ? configuration.locale - : vscode.env.language); - function getDignosticsKind(event: Proto.Event) { switch (event.event) { case 'syntaxDiag': return DiagnosticKind.Syntax; diff --git a/extensions/typescript-language-features/src/utils/electron.ts b/extensions/typescript-language-features/src/utils/electron.ts index b6ed76ceea0..da11362b419 100644 --- a/extensions/typescript-language-features/src/utils/electron.ts +++ b/extensions/typescript-language-features/src/utils/electron.ts @@ -3,16 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import Logger from './logger'; import * as temp from './temp'; import path = require('path'); import fs = require('fs'); import cp = require('child_process'); -export interface IForkOptions { - cwd?: string; - execArgv?: string[]; -} const getRootTempDir = (() => { let dir: string | undefined; @@ -31,29 +26,29 @@ export function getTempFile(prefix: string): string { return path.join(getRootTempDir(), `${prefix}-${temp.makeRandomHexString(20)}.tmp`); } -function generatePatchedEnv(env: any): any { +function generatePatchedEnv(env: any, modulePath: string): any { const newEnv = Object.assign({}, env); - // Set the two unique pipe names and the electron flag as process env newEnv['ELECTRON_RUN_AS_NODE'] = '1'; + newEnv['NODE_PATH'] = path.join(modulePath, '..', '..', '..'); // Ensure we always have a PATH set newEnv['PATH'] = newEnv['PATH'] || process.env.PATH; + return newEnv; } +export interface IForkOptions { + readonly cwd?: string; + readonly execArgv?: string[]; +} + export function fork( modulePath: string, args: string[], options: IForkOptions, - logger: Logger ): cp.ChildProcess { - const newEnv = generatePatchedEnv(process.env); - newEnv['NODE_PATH'] = path.join(modulePath, '..', '..', '..'); - - // Create the process - logger.info('Forking TSServer', `PATH: ${newEnv['PATH']} `); - + const newEnv = generatePatchedEnv(process.env, modulePath); return cp.fork(modulePath, args, { silent: true, cwd: options.cwd, diff --git a/extensions/typescript-language-features/src/utils/logDirectoryProvider.ts b/extensions/typescript-language-features/src/utils/logDirectoryProvider.ts index 5f97d445a90..af6886e7043 100644 --- a/extensions/typescript-language-features/src/utils/logDirectoryProvider.ts +++ b/extensions/typescript-language-features/src/utils/logDirectoryProvider.ts @@ -13,7 +13,7 @@ export default class LogDirectoryProvider { private readonly context: vscode.ExtensionContext ) { } - public async getNewLogDirectory(): Promise { + public getNewLogDirectory(): string | undefined { const root = this.logDirectory(); if (root) { try { diff --git a/src/vs/workbench/parts/webview/electron-browser/webview-pre.js b/src/vs/workbench/parts/webview/electron-browser/webview-pre.js index 17e1b20b3f4..50e67535d00 100644 --- a/src/vs/workbench/parts/webview/electron-browser/webview-pre.js +++ b/src/vs/workbench/parts/webview/electron-browser/webview-pre.js @@ -361,9 +361,6 @@ // Workaround for https://github.com/Microsoft/vscode/issues/12865 // check new scrollTop and reset if neccessary setInitialScrollPosition(contentDocument.body, contentWindow); - - // Bubble out link clicks - contentDocument.body.addEventListener('click', handleInnerClick); } const newFrame = getPendingFrame(); @@ -401,6 +398,9 @@ } }); + // Bubble out link clicks + newFrame.contentWindow.addEventListener('click', handleInnerClick); + // set DOCTYPE for newDocument explicitly as DOMParser.parseFromString strips it off // and DOCTYPE is needed in the iframe to ensure that the user agent stylesheet is correctly overridden newFrame.contentDocument.write('');