Skip to content

Commit 619ab6d

Browse files
authored
Merge pull request #153024 from microsoft/alexd/ext-host-console-2
Clean up extension host console forwarding
2 parents 26edd1f + bbee9d7 commit 619ab6d

File tree

11 files changed

+195
-289
lines changed

11 files changed

+195
-289
lines changed

src/vs/base/common/console.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export interface IRemoteConsoleLog {
1111
arguments: string;
1212
}
1313

14-
interface IStackArgument {
14+
export interface IStackArgument {
1515
__$stack: string;
1616
}
1717

src/vs/server/node/extensionHostConnection.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { getNLSConfiguration } from 'vs/server/node/remoteLanguagePacks';
99
import { FileAccess } from 'vs/base/common/network';
1010
import { join, delimiter } from 'vs/base/common/path';
1111
import { VSBuffer } from 'vs/base/common/buffer';
12-
import { IRemoteConsoleLog } from 'vs/base/common/console';
1312
import { Emitter, Event } from 'vs/base/common/event';
1413
import { createRandomIPCHandle, NodeSocket, WebSocketNodeSocket } from 'vs/base/parts/ipc/node/ipc.net';
1514
import { getResolvedShellEnv } from 'vs/platform/shell/node/shellEnv';
@@ -18,13 +17,12 @@ import { IRemoteExtensionHostStartParams } from 'vs/platform/remote/common/remot
1817
import { IExtHostReadyMessage, IExtHostSocketMessage, IExtHostReduceGraceTimeMessage } from 'vs/workbench/services/extensions/common/extensionHostProtocol';
1918
import { IServerEnvironmentService } from 'vs/server/node/serverEnvironmentService';
2019
import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
21-
import { logRemoteEntry } from 'vs/workbench/services/extensions/common/remoteConsoleUtil';
2220
import { removeDangerousEnvVariables } from 'vs/base/common/processes';
2321
import { IExtensionHostStatusService } from 'vs/server/node/extensionHostStatusService';
2422
import { DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
2523
import { IPCExtHostConnection, writeExtHostConnection, SocketExtHostConnection } from 'vs/workbench/services/extensions/common/extensionHostEnv';
2624

27-
export async function buildUserEnvironment(startParamsEnv: { [key: string]: string | null } = {}, withUserShellEnvironment: boolean, language: string, isDebug: boolean, environmentService: IServerEnvironmentService, logService: ILogService): Promise<IProcessEnvironment> {
25+
export async function buildUserEnvironment(startParamsEnv: { [key: string]: string | null } = {}, withUserShellEnvironment: boolean, language: string, environmentService: IServerEnvironmentService, logService: ILogService): Promise<IProcessEnvironment> {
2826
const nlsConfig = await getNLSConfiguration(language, environmentService.userDataPath);
2927

3028
let userShellEnv: typeof process.env = {};
@@ -42,11 +40,8 @@ export async function buildUserEnvironment(startParamsEnv: { [key: string]: stri
4240
...processEnv,
4341
...userShellEnv,
4442
...{
45-
VSCODE_LOG_NATIVE: String(isDebug),
4643
VSCODE_AMD_ENTRYPOINT: 'vs/workbench/api/node/extensionHostProcess',
47-
VSCODE_VERBOSE_LOGGING: 'true',
4844
VSCODE_HANDLES_UNCAUGHT_ERRORS: 'true',
49-
VSCODE_LOG_STACK: 'false',
5045
VSCODE_NLS_CONFIG: JSON.stringify(nlsConfig, undefined, 0)
5146
},
5247
...startParamsEnv
@@ -238,7 +233,7 @@ export class ExtensionHostConnection {
238233
execArgv = [`--inspect${startParams.break ? '-brk' : ''}=${startParams.port}`];
239234
}
240235

241-
const env = await buildUserEnvironment(startParams.env, true, startParams.language, !!startParams.debugId, this._environmentService, this._logService);
236+
const env = await buildUserEnvironment(startParams.env, true, startParams.language, this._environmentService, this._logService);
242237
removeDangerousEnvVariables(env);
243238

244239
let extHostNamedPipeServer: net.Server | null;
@@ -274,14 +269,6 @@ export class ExtensionHostConnection {
274269
onStdout((e) => this._log(`<${pid}> ${e}`));
275270
onStderr((e) => this._log(`<${pid}><stderr> ${e}`));
276271

277-
278-
// Support logging from extension host
279-
this._extensionHostProcess.on('message', msg => {
280-
if (msg && (<IRemoteConsoleLog>msg).type === '__$console') {
281-
logRemoteEntry(this._logService, (<IRemoteConsoleLog>msg), `${this._logPrefix}<${pid}>`);
282-
}
283-
});
284-
285272
// Lifecycle
286273
this._extensionHostProcess.on('error', (err) => {
287274
this._logError(`<${pid}> Extension Host Process had an error`);

src/vs/server/node/remoteTerminalChannel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export class RemoteTerminalChannel extends Disposable implements IServerChannel<
188188
};
189189

190190

191-
const baseEnv = await buildUserEnvironment(args.resolverEnv, !!args.shellLaunchConfig.useShellEnvironment, platform.language, false, this._environmentService, this._logService);
191+
const baseEnv = await buildUserEnvironment(args.resolverEnv, !!args.shellLaunchConfig.useShellEnvironment, platform.language, this._environmentService, this._logService);
192192
this._logService.trace('baseEnv', baseEnv);
193193

194194
const reviveWorkspaceFolder = (workspaceData: IWorkspaceFolderData): IWorkspaceFolder => {
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { IStackArgument } from 'vs/base/common/console';
7+
import { safeStringify } from 'vs/base/common/objects';
8+
import { MainContext, MainThreadConsoleShape } from 'vs/workbench/api/common/extHost.protocol';
9+
import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService';
10+
import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
11+
12+
export abstract class AbstractExtHostConsoleForwarder {
13+
14+
private readonly _mainThreadConsole: MainThreadConsoleShape;
15+
private readonly _includeStack: boolean;
16+
private readonly _logNative: boolean;
17+
18+
constructor(
19+
@IExtHostRpcService extHostRpc: IExtHostRpcService,
20+
@IExtHostInitDataService initData: IExtHostInitDataService,
21+
) {
22+
this._mainThreadConsole = extHostRpc.getProxy(MainContext.MainThreadConsole);
23+
this._includeStack = initData.consoleForward.includeStack;
24+
this._logNative = initData.consoleForward.logNative;
25+
26+
// Pass console logging to the outside so that we have it in the main side if told so
27+
this._wrapConsoleMethod('info', 'log');
28+
this._wrapConsoleMethod('log', 'log');
29+
this._wrapConsoleMethod('warn', 'warn');
30+
this._wrapConsoleMethod('error', 'error');
31+
}
32+
33+
/**
34+
* Wraps a console message so that it is transmitted to the renderer. If
35+
* native logging is turned on, the original console message will be written
36+
* as well. This is needed since the console methods are "magic" in V8 and
37+
* are the only methods that allow later introspection of logged variables.
38+
*
39+
* The wrapped property is not defined with `writable: false` to avoid
40+
* throwing errors, but rather a no-op setting. See https://github.com/microsoft/vscode-extension-telemetry/issues/88
41+
*/
42+
private _wrapConsoleMethod(method: 'log' | 'info' | 'warn' | 'error', severity: 'log' | 'warn' | 'error') {
43+
const that = this;
44+
const original = console[method];
45+
46+
Object.defineProperty(console, method, {
47+
set: () => { },
48+
get: () => function () {
49+
that._handleConsoleCall(method, severity, original, arguments);
50+
},
51+
});
52+
}
53+
54+
private _handleConsoleCall(method: 'log' | 'info' | 'warn' | 'error', severity: 'log' | 'warn' | 'error', original: (...args: any[]) => void, args: IArguments): void {
55+
this._mainThreadConsole.$logExtensionHostMessage({
56+
type: '__$console',
57+
severity,
58+
arguments: safeStringifyArgumentsToArray(args, this._includeStack)
59+
});
60+
if (this._logNative) {
61+
this._nativeConsoleLogMessage(method, original, args);
62+
}
63+
}
64+
65+
protected abstract _nativeConsoleLogMessage(method: 'log' | 'info' | 'warn' | 'error', original: (...args: any[]) => void, args: IArguments): void;
66+
67+
}
68+
69+
const MAX_LENGTH = 100000;
70+
71+
/**
72+
* Prevent circular stringify and convert arguments to real array
73+
*/
74+
function safeStringifyArgumentsToArray(args: IArguments, includeStack: boolean): string {
75+
const argsArray = [];
76+
77+
// Massage some arguments with special treatment
78+
if (args.length) {
79+
for (let i = 0; i < args.length; i++) {
80+
let arg = args[i];
81+
82+
// Any argument of type 'undefined' needs to be specially treated because
83+
// JSON.stringify will simply ignore those. We replace them with the string
84+
// 'undefined' which is not 100% right, but good enough to be logged to console
85+
if (typeof arg === 'undefined') {
86+
arg = 'undefined';
87+
}
88+
89+
// Any argument that is an Error will be changed to be just the error stack/message
90+
// itself because currently cannot serialize the error over entirely.
91+
else if (arg instanceof Error) {
92+
const errorObj = arg;
93+
if (errorObj.stack) {
94+
arg = errorObj.stack;
95+
} else {
96+
arg = errorObj.toString();
97+
}
98+
}
99+
100+
argsArray.push(arg);
101+
}
102+
}
103+
104+
// Add the stack trace as payload if we are told so. We remove the message and the 2 top frames
105+
// to start the stacktrace where the console message was being written
106+
if (includeStack) {
107+
const stack = new Error().stack;
108+
if (stack) {
109+
argsArray.push({ __$stack: stack.split('\n').slice(3).join('\n') } as IStackArgument);
110+
}
111+
}
112+
113+
try {
114+
const res = safeStringify(argsArray);
115+
116+
if (res.length > MAX_LENGTH) {
117+
return 'Output omitted for a large object that exceeds the limits';
118+
}
119+
120+
return res;
121+
} catch (error) {
122+
return `Output omitted for an object that cannot be inspected ('${error.toString()}')`;
123+
}
124+
}

0 commit comments

Comments
 (0)