Skip to content

Commit

Permalink
console: move the inspector console wrapping in a separate file
Browse files Browse the repository at this point in the history
Move the wrapping of the inspector console in a separate file
for clarity. In addition, save the original console from the
VM explicitly via an exported property
`require('internal/console/inspector').consoleFromVM`
that `require('inspector').console` can alias to it later,
instead of hanging the original console onto `per_thread.js`
during bootstrap and counting on that `per_thread.js`
only gets evaluated once and gets cached.

PR-URL: #24709
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and addaleax committed Jan 14, 2019
1 parent b549058 commit 57323e8
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 55 deletions.
5 changes: 3 additions & 2 deletions lib/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const {
const { validateString } = require('internal/validators');
const util = require('util');
const { Connection, open, url } = internalBinding('inspector');
const { originalConsole } = require('internal/process/per_thread');

if (!Connection)
throw new ERR_INSPECTOR_NOT_AVAILABLE();
Expand Down Expand Up @@ -103,6 +102,8 @@ module.exports = {
open: (port, host, wait) => open(port, host, !!wait),
close: process._debugEnd,
url: url,
console: originalConsole,
// This is dynamically added during bootstrap,
// where the console from the VM is still available
console: require('internal/console/inspector').consoleFromVM,
Session
};
68 changes: 15 additions & 53 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@

const browserGlobals = !process._noBrowserGlobals;
if (browserGlobals) {
// We are setting this here to forward it to the inspector later
perThreadSetup.originalConsole = global.console;
setupGlobalTimeouts();
setupGlobalConsole();
setupGlobalURL();
Expand Down Expand Up @@ -486,16 +484,25 @@
}

function setupGlobalConsole() {
const originalConsole = global.console;
// Setup Node.js global.console.
const wrappedConsole = NativeModule.require('console');
const consoleFromVM = global.console;
const consoleFromNode =
NativeModule.require('internal/console/global');
// Override global console from the one provided by the VM
// to the one implemented by Node.js
Object.defineProperty(global, 'console', {
configurable: true,
enumerable: false,
value: wrappedConsole,
value: consoleFromNode,
writable: true
});
setupInspector(originalConsole, wrappedConsole);
// TODO(joyeecheung): can we skip this if inspector is not active?
if (process.config.variables.v8_enable_inspector) {
const inspector =
NativeModule.require('internal/console/inspector');
inspector.addInspectorApis(consoleFromNode, consoleFromVM);
// This will be exposed by `require('inspector').console` later.
inspector.consoleFromVM = consoleFromVM;
}
}

function setupGlobalURL() {
Expand Down Expand Up @@ -570,41 +577,6 @@
registerDOMException(DOMException);
}

function setupInspector(originalConsole, wrappedConsole) {
if (!process.config.variables.v8_enable_inspector) {
return;
}
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
const { addCommandLineAPI, consoleCall } = process.binding('inspector');
// Setup inspector command line API.
const { makeRequireFunction } =
NativeModule.require('internal/modules/cjs/helpers');
const path = NativeModule.require('path');
const cwd = tryGetCwd(path);

const consoleAPIModule = new CJSModule('<inspector console>');
consoleAPIModule.paths =
CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
addCommandLineAPI('require', makeRequireFunction(consoleAPIModule));
const config = {};
for (const key of Object.keys(wrappedConsole)) {
if (!originalConsole.hasOwnProperty(key))
continue;
// If global console has the same method as inspector console,
// then wrap these two methods into one. Native wrapper will preserve
// the original stack.
wrappedConsole[key] = consoleCall.bind(wrappedConsole,
originalConsole[key],
wrappedConsole[key],
config);
}
for (const key of Object.keys(originalConsole)) {
if (wrappedConsole.hasOwnProperty(key))
continue;
wrappedConsole[key] = originalConsole[key];
}
}

function noop() {}

function setupProcessFatal() {
Expand Down Expand Up @@ -683,17 +655,6 @@
}
}

function tryGetCwd(path) {
try {
return process.cwd();
} catch {
// getcwd(3) can fail if the current working directory has been deleted.
// Fall back to the directory name of the (absolute) executable path.
// It's not really correct but what are the alternatives?
return path.dirname(process.execPath);
}
}

function wrapForBreakOnFirstLine(source) {
if (!process._breakFirstLine)
return source;
Expand All @@ -704,6 +665,7 @@
function evalScript(name, body) {
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
const path = NativeModule.require('path');
const { tryGetCwd } = NativeModule.require('internal/util');
const cwd = tryGetCwd(path);

const module = new CJSModule(name);
Expand Down
53 changes: 53 additions & 0 deletions lib/internal/console/inspector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

const path = require('path');
const CJSModule = require('internal/modules/cjs/loader');
const { makeRequireFunction } = require('internal/modules/cjs/helpers');
const { tryGetCwd } = require('internal/util');
const { addCommandLineAPI, consoleCall } = process.binding('inspector');

// Wrap a console implemented by Node.js with features from the VM inspector
function addInspectorApis(consoleFromNode, consoleFromVM) {
// Setup inspector command line API.
const cwd = tryGetCwd(path);
const consoleAPIModule = new CJSModule('<inspector console>');
consoleAPIModule.paths =
CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
addCommandLineAPI('require', makeRequireFunction(consoleAPIModule));
const config = {};

// If global console has the same method as inspector console,
// then wrap these two methods into one. Native wrapper will preserve
// the original stack.
for (const key of Object.keys(consoleFromNode)) {
if (!consoleFromVM.hasOwnProperty(key))
continue;
consoleFromNode[key] = consoleCall.bind(consoleFromNode,
consoleFromVM[key],
consoleFromNode[key],
config);
}

// Add additional console APIs from the inspector
for (const key of Object.keys(consoleFromVM)) {
if (consoleFromNode.hasOwnProperty(key))
continue;
consoleFromNode[key] = consoleFromVM[key];
}
}

module.exports = {
addInspectorApis
};

// Stores the console from VM, should be set during bootstrap.
let consoleFromVM;

Object.defineProperty(module.exports, 'consoleFromVM', {
get() {
return consoleFromVM;
},
set(val) {
consoleFromVM = val;
}
});
12 changes: 12 additions & 0 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,17 @@ function once(callback) {
};
}

function tryGetCwd(path) {
try {
return process.cwd();
} catch {
// getcwd(3) can fail if the current working directory has been deleted.
// Fall back to the directory name of the (absolute) executable path.
// It's not really correct but what are the alternatives?
return path.dirname(process.execPath);
}
}

module.exports = {
assertCrypto,
cachedResult,
Expand All @@ -406,6 +417,7 @@ module.exports = {
once,
promisify,
spliceOne,
tryGetCwd,
removeColors,

// Symbol used to customize promisify conversion
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
'lib/internal/cluster/worker.js',
'lib/internal/console/constructor.js',
'lib/internal/console/global.js',
'lib/internal/console/inspector.js',
'lib/internal/crypto/certificate.js',
'lib/internal/crypto/cipher.js',
'lib/internal/crypto/diffiehellman.js',
Expand Down

0 comments on commit 57323e8

Please sign in to comment.