Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 57323e8

Browse filesBrowse files
joyeecheungaddaleax
joyeecheung
authored and
addaleax
committed
console: move the inspector console wrapping in a separate file
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>
1 parent b549058 commit 57323e8
Copy full SHA for 57323e8

File tree

Expand file treeCollapse file tree

5 files changed

+84
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+84
-55
lines changed
Open diff view settings
Collapse file

‎lib/inspector.js‎

Copy file name to clipboardExpand all lines: lib/inspector.js
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ const {
1212
const { validateString } = require('internal/validators');
1313
const util = require('util');
1414
const { Connection, open, url } = internalBinding('inspector');
15-
const { originalConsole } = require('internal/process/per_thread');
1615

1716
if (!Connection)
1817
throw new ERR_INSPECTOR_NOT_AVAILABLE();
@@ -103,6 +102,8 @@ module.exports = {
103102
open: (port, host, wait) => open(port, host, !!wait),
104103
close: process._debugEnd,
105104
url: url,
106-
console: originalConsole,
105+
// This is dynamically added during bootstrap,
106+
// where the console from the VM is still available
107+
console: require('internal/console/inspector').consoleFromVM,
107108
Session
108109
};
Collapse file

‎lib/internal/bootstrap/node.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/node.js
+15-53Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,6 @@
121121

122122
const browserGlobals = !process._noBrowserGlobals;
123123
if (browserGlobals) {
124-
// We are setting this here to forward it to the inspector later
125-
perThreadSetup.originalConsole = global.console;
126124
setupGlobalTimeouts();
127125
setupGlobalConsole();
128126
setupGlobalURL();
@@ -486,16 +484,25 @@
486484
}
487485

488486
function setupGlobalConsole() {
489-
const originalConsole = global.console;
490-
// Setup Node.js global.console.
491-
const wrappedConsole = NativeModule.require('console');
487+
const consoleFromVM = global.console;
488+
const consoleFromNode =
489+
NativeModule.require('internal/console/global');
490+
// Override global console from the one provided by the VM
491+
// to the one implemented by Node.js
492492
Object.defineProperty(global, 'console', {
493493
configurable: true,
494494
enumerable: false,
495-
value: wrappedConsole,
495+
value: consoleFromNode,
496496
writable: true
497497
});
498-
setupInspector(originalConsole, wrappedConsole);
498+
// TODO(joyeecheung): can we skip this if inspector is not active?
499+
if (process.config.variables.v8_enable_inspector) {
500+
const inspector =
501+
NativeModule.require('internal/console/inspector');
502+
inspector.addInspectorApis(consoleFromNode, consoleFromVM);
503+
// This will be exposed by `require('inspector').console` later.
504+
inspector.consoleFromVM = consoleFromVM;
505+
}
499506
}
500507

501508
function setupGlobalURL() {
@@ -570,41 +577,6 @@
570577
registerDOMException(DOMException);
571578
}
572579

573-
function setupInspector(originalConsole, wrappedConsole) {
574-
if (!process.config.variables.v8_enable_inspector) {
575-
return;
576-
}
577-
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
578-
const { addCommandLineAPI, consoleCall } = process.binding('inspector');
579-
// Setup inspector command line API.
580-
const { makeRequireFunction } =
581-
NativeModule.require('internal/modules/cjs/helpers');
582-
const path = NativeModule.require('path');
583-
const cwd = tryGetCwd(path);
584-
585-
const consoleAPIModule = new CJSModule('<inspector console>');
586-
consoleAPIModule.paths =
587-
CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
588-
addCommandLineAPI('require', makeRequireFunction(consoleAPIModule));
589-
const config = {};
590-
for (const key of Object.keys(wrappedConsole)) {
591-
if (!originalConsole.hasOwnProperty(key))
592-
continue;
593-
// If global console has the same method as inspector console,
594-
// then wrap these two methods into one. Native wrapper will preserve
595-
// the original stack.
596-
wrappedConsole[key] = consoleCall.bind(wrappedConsole,
597-
originalConsole[key],
598-
wrappedConsole[key],
599-
config);
600-
}
601-
for (const key of Object.keys(originalConsole)) {
602-
if (wrappedConsole.hasOwnProperty(key))
603-
continue;
604-
wrappedConsole[key] = originalConsole[key];
605-
}
606-
}
607-
608580
function noop() {}
609581

610582
function setupProcessFatal() {
@@ -683,17 +655,6 @@
683655
}
684656
}
685657

686-
function tryGetCwd(path) {
687-
try {
688-
return process.cwd();
689-
} catch {
690-
// getcwd(3) can fail if the current working directory has been deleted.
691-
// Fall back to the directory name of the (absolute) executable path.
692-
// It's not really correct but what are the alternatives?
693-
return path.dirname(process.execPath);
694-
}
695-
}
696-
697658
function wrapForBreakOnFirstLine(source) {
698659
if (!process._breakFirstLine)
699660
return source;
@@ -704,6 +665,7 @@
704665
function evalScript(name, body) {
705666
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
706667
const path = NativeModule.require('path');
668+
const { tryGetCwd } = NativeModule.require('internal/util');
707669
const cwd = tryGetCwd(path);
708670

709671
const module = new CJSModule(name);
Collapse file

‎lib/internal/console/inspector.js‎

Copy file name to clipboard
+53Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
3+
const path = require('path');
4+
const CJSModule = require('internal/modules/cjs/loader');
5+
const { makeRequireFunction } = require('internal/modules/cjs/helpers');
6+
const { tryGetCwd } = require('internal/util');
7+
const { addCommandLineAPI, consoleCall } = process.binding('inspector');
8+
9+
// Wrap a console implemented by Node.js with features from the VM inspector
10+
function addInspectorApis(consoleFromNode, consoleFromVM) {
11+
// Setup inspector command line API.
12+
const cwd = tryGetCwd(path);
13+
const consoleAPIModule = new CJSModule('<inspector console>');
14+
consoleAPIModule.paths =
15+
CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
16+
addCommandLineAPI('require', makeRequireFunction(consoleAPIModule));
17+
const config = {};
18+
19+
// If global console has the same method as inspector console,
20+
// then wrap these two methods into one. Native wrapper will preserve
21+
// the original stack.
22+
for (const key of Object.keys(consoleFromNode)) {
23+
if (!consoleFromVM.hasOwnProperty(key))
24+
continue;
25+
consoleFromNode[key] = consoleCall.bind(consoleFromNode,
26+
consoleFromVM[key],
27+
consoleFromNode[key],
28+
config);
29+
}
30+
31+
// Add additional console APIs from the inspector
32+
for (const key of Object.keys(consoleFromVM)) {
33+
if (consoleFromNode.hasOwnProperty(key))
34+
continue;
35+
consoleFromNode[key] = consoleFromVM[key];
36+
}
37+
}
38+
39+
module.exports = {
40+
addInspectorApis
41+
};
42+
43+
// Stores the console from VM, should be set during bootstrap.
44+
let consoleFromVM;
45+
46+
Object.defineProperty(module.exports, 'consoleFromVM', {
47+
get() {
48+
return consoleFromVM;
49+
},
50+
set(val) {
51+
consoleFromVM = val;
52+
}
53+
});
Collapse file

‎lib/internal/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/util.js
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,17 @@ function once(callback) {
387387
};
388388
}
389389

390+
function tryGetCwd(path) {
391+
try {
392+
return process.cwd();
393+
} catch {
394+
// getcwd(3) can fail if the current working directory has been deleted.
395+
// Fall back to the directory name of the (absolute) executable path.
396+
// It's not really correct but what are the alternatives?
397+
return path.dirname(process.execPath);
398+
}
399+
}
400+
390401
module.exports = {
391402
assertCrypto,
392403
cachedResult,
@@ -406,6 +417,7 @@ module.exports = {
406417
once,
407418
promisify,
408419
spliceOne,
420+
tryGetCwd,
409421
removeColors,
410422

411423
// Symbol used to customize promisify conversion
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
'lib/internal/cluster/worker.js',
9999
'lib/internal/console/constructor.js',
100100
'lib/internal/console/global.js',
101+
'lib/internal/console/inspector.js',
101102
'lib/internal/crypto/certificate.js',
102103
'lib/internal/crypto/cipher.js',
103104
'lib/internal/crypto/diffiehellman.js',

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.