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 010f864

Browse filesBrowse files
refackjasnell
authored andcommitted
inspector: --debug* deprecation and invalidation
PR-URL: #12949 Fixes: #12364 Fixes: #12630 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jan Krems <jan.krems@gmail.com>
1 parent 5077cde commit 010f864
Copy full SHA for 010f864

File tree

Expand file treeCollapse file tree

10 files changed

+180
-36
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+180
-36
lines changed
Open diff view settings
Collapse file

‎lib/internal/bootstrap_node.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap_node.js
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,20 @@
6565
});
6666
process.argv[0] = process.execPath;
6767

68+
// Handle `--debug*` deprecation and invalidation
69+
if (process._invalidDebug) {
70+
process.emitWarning(
71+
'`node --debug` and `node --debug-brk` are invalid. ' +
72+
'Please use `node --inspect` or `node --inspect-brk` instead.',
73+
'DeprecationWarning', 'DEP0062', startup, true);
74+
process.exit(9);
75+
} else if (process._deprecatedDebugBrk) {
76+
process.emitWarning(
77+
'`node --inspect --debug-brk` is deprecated. ' +
78+
'Please use `node --inspect-brk` instead.',
79+
'DeprecationWarning', 'DEP0062', startup, true);
80+
}
81+
6882
// There are various modes that Node can run in. The most common two
6983
// are running from a script and running the REPL - but there are a few
7084
// others like the debugger or running --eval arguments. Here we decide
Collapse file

‎lib/internal/process/warning.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/warning.js
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ function setupProcessWarnings() {
111111
// process.emitWarning(error)
112112
// process.emitWarning(str[, type[, code]][, ctor])
113113
// process.emitWarning(str[, options])
114-
process.emitWarning = function(warning, type, code, ctor) {
114+
process.emitWarning = function(warning, type, code, ctor, now) {
115115
const errors = lazyErrors();
116116
var detail;
117117
if (type !== null && typeof type === 'object' && !Array.isArray(type)) {
@@ -150,6 +150,7 @@ function setupProcessWarnings() {
150150
if (process.throwDeprecation)
151151
throw warning;
152152
}
153-
process.nextTick(doEmitWarning(warning));
153+
if (now) process.emit('warning', warning);
154+
else process.nextTick(doEmitWarning(warning));
154155
};
155156
}
Collapse file

‎lib/module.js‎

Copy file name to clipboardExpand all lines: lib/module.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ Module.prototype._compile = function(content, filename) {
537537
});
538538

539539
var inspectorWrapper = null;
540-
if (process._debugWaitConnect && process._eval == null) {
540+
if (process._breakFirstLine && process._eval == null) {
541541
if (!resolvedArgv) {
542542
// we enter the repl if we're not given a filename argument.
543543
if (process.argv[1]) {
@@ -549,7 +549,7 @@ Module.prototype._compile = function(content, filename) {
549549

550550
// Set breakpoint on module start
551551
if (filename === resolvedArgv) {
552-
delete process._debugWaitConnect;
552+
delete process._breakFirstLine;
553553
inspectorWrapper = process.binding('inspector').callAndPauseOnStart;
554554
if (!inspectorWrapper) {
555555
const Debug = vm.runInDebugContext('Debug');
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+21-1Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3391,9 +3391,29 @@ void SetupProcessObject(Environment* env,
33913391
READONLY_PROPERTY(process, "traceDeprecation", True(env->isolate()));
33923392
}
33933393

3394+
// TODO(refack): move the following 4 to `node_config`
3395+
// --inspect
3396+
if (debug_options.inspector_enabled()) {
3397+
READONLY_DONT_ENUM_PROPERTY(process,
3398+
"_inspectorEnbale", True(env->isolate()));
3399+
}
3400+
33943401
// --inspect-brk
33953402
if (debug_options.wait_for_connect()) {
3396-
READONLY_PROPERTY(process, "_debugWaitConnect", True(env->isolate()));
3403+
READONLY_DONT_ENUM_PROPERTY(process,
3404+
"_breakFirstLine", True(env->isolate()));
3405+
}
3406+
3407+
// --inspect --debug-brk
3408+
if (debug_options.deprecated_invocation()) {
3409+
READONLY_DONT_ENUM_PROPERTY(process,
3410+
"_deprecatedDebugBrk", True(env->isolate()));
3411+
}
3412+
3413+
// --debug or, --debug-brk without --inspect
3414+
if (debug_options.invalid_invocation()) {
3415+
READONLY_DONT_ENUM_PROPERTY(process,
3416+
"_invalidDebug", True(env->isolate()));
33973417
}
33983418

33993419
// --security-revert flags
Collapse file

‎src/node_debug_options.cc‎

Copy file name to clipboardExpand all lines: src/node_debug_options.cc
+20-17Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
namespace node {
99

1010
namespace {
11-
#if HAVE_INSPECTOR
1211
const int default_inspector_port = 9229;
13-
#endif // HAVE_INSPECTOR
1412

1513
inline std::string remove_brackets(const std::string& host) {
1614
if (!host.empty() && host.front() == '[' && host.back() == ']')
@@ -56,14 +54,12 @@ std::pair<std::string, int> split_host_port(const std::string& arg) {
5654
} // namespace
5755

5856
DebugOptions::DebugOptions() :
59-
#if HAVE_INSPECTOR
6057
inspector_enabled_(false),
61-
#endif // HAVE_INSPECTOR
62-
wait_connect_(false), http_enabled_(false),
58+
deprecated_debug_(false),
59+
break_first_line_(false),
6360
host_name_("127.0.0.1"), port_(-1) { }
6461

6562
bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
66-
bool enable_inspector = false;
6763
bool has_argument = false;
6864
std::string option_name;
6965
std::string argument;
@@ -81,11 +77,21 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
8177
argument.clear();
8278
}
8379

80+
// Note that --debug-port and --debug-brk in conjuction with --inspect
81+
// work but are undocumented.
82+
// --debug is no longer valid.
83+
// Ref: https://github.com/nodejs/node/issues/12630
84+
// Ref: https://github.com/nodejs/node/pull/12949
8485
if (option_name == "--inspect") {
85-
enable_inspector = true;
86+
inspector_enabled_ = true;
87+
} else if (option_name == "--debug") {
88+
deprecated_debug_ = true;
8689
} else if (option_name == "--inspect-brk") {
87-
enable_inspector = true;
88-
wait_connect_ = true;
90+
inspector_enabled_ = true;
91+
break_first_line_ = true;
92+
} else if (option_name == "--debug-brk") {
93+
break_first_line_ = true;
94+
deprecated_debug_ = true;
8995
} else if (option_name == "--debug-port" ||
9096
option_name == "--inspect-port") {
9197
if (!has_argument) {
@@ -97,15 +103,14 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
97103
return false;
98104
}
99105

100-
if (enable_inspector) {
101-
#if HAVE_INSPECTOR
102-
inspector_enabled_ = true;
103-
#else
106+
#if !HAVE_INSPECTOR
107+
if (inspector_enabled_) {
104108
fprintf(stderr,
105109
"Inspector support is not available with this Node.js build\n");
106-
return false;
107-
#endif
108110
}
111+
inspector_enabled_ = false;
112+
return false;
113+
#endif
109114

110115
// argument can be specified for *any* option to specify host:port
111116
if (has_argument) {
@@ -124,9 +129,7 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
124129
int DebugOptions::port() const {
125130
int port = port_;
126131
if (port < 0) {
127-
#if HAVE_INSPECTOR
128132
port = default_inspector_port;
129-
#endif // HAVE_INSPECTOR
130133
}
131134
return port;
132135
}
Collapse file

‎src/node_debug_options.h‎

Copy file name to clipboardExpand all lines: src/node_debug_options.h
+11-12Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,24 @@ class DebugOptions {
1010
public:
1111
DebugOptions();
1212
bool ParseOption(const char* argv0, const std::string& option);
13-
bool inspector_enabled() const {
14-
#if HAVE_INSPECTOR
15-
return inspector_enabled_;
16-
#else
17-
return false;
18-
#endif // HAVE_INSPECTOR
13+
bool inspector_enabled() const { return inspector_enabled_; }
14+
bool deprecated_invocation() const {
15+
return deprecated_debug_ &&
16+
inspector_enabled_ &&
17+
break_first_line_;
1918
}
20-
bool ToolsServerEnabled();
21-
bool wait_for_connect() const { return wait_connect_; }
19+
bool invalid_invocation() const {
20+
return deprecated_debug_ && !inspector_enabled_;
21+
}
22+
bool wait_for_connect() const { return break_first_line_; }
2223
std::string host_name() const { return host_name_; }
2324
int port() const;
2425
void set_port(int port) { port_ = port; }
2526

2627
private:
27-
#if HAVE_INSPECTOR
2828
bool inspector_enabled_;
29-
#endif // HAVE_INSPECTOR
30-
bool wait_connect_;
31-
bool http_enabled_;
29+
bool deprecated_debug_;
30+
bool break_first_line_;
3231
std::string host_name_;
3332
int port_;
3433
};
Collapse file

‎test/inspector/inspector-helper.js‎

Copy file name to clipboardExpand all lines: test/inspector/inspector-helper.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,9 +496,9 @@ Harness.prototype.kill = function() {
496496
};
497497

498498
exports.startNodeForInspectorTest = function(callback,
499-
inspectorFlag = '--inspect-brk',
499+
inspectorFlags = ['--inspect-brk'],
500500
opt_script_contents) {
501-
const args = [inspectorFlag];
501+
const args = [].concat(inspectorFlags);
502502
if (opt_script_contents) {
503503
args.push('-e', opt_script_contents);
504504
} else {
Collapse file
+57Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
const common = require('../common');
3+
common.skipIfInspectorDisabled();
4+
const assert = require('assert');
5+
const helper = require('./inspector-helper.js');
6+
7+
function setupExpectBreakOnLine(line, url, session, scopeIdCallback) {
8+
return function(message) {
9+
if ('Debugger.paused' === message['method']) {
10+
const callFrame = message['params']['callFrames'][0];
11+
const location = callFrame['location'];
12+
assert.strictEqual(url, session.scriptUrlForId(location['scriptId']));
13+
assert.strictEqual(line, location['lineNumber']);
14+
scopeIdCallback &&
15+
scopeIdCallback(callFrame['scopeChain'][0]['object']['objectId']);
16+
return true;
17+
}
18+
};
19+
}
20+
21+
function testBreakpointOnStart(session) {
22+
const commands = [
23+
{ 'method': 'Runtime.enable' },
24+
{ 'method': 'Debugger.enable' },
25+
{ 'method': 'Debugger.setPauseOnExceptions',
26+
'params': {'state': 'none'} },
27+
{ 'method': 'Debugger.setAsyncCallStackDepth',
28+
'params': {'maxDepth': 0} },
29+
{ 'method': 'Profiler.enable' },
30+
{ 'method': 'Profiler.setSamplingInterval',
31+
'params': {'interval': 100} },
32+
{ 'method': 'Debugger.setBlackboxPatterns',
33+
'params': {'patterns': []} },
34+
{ 'method': 'Runtime.runIfWaitingForDebugger' }
35+
];
36+
37+
session
38+
.sendInspectorCommands(commands)
39+
.expectMessages(setupExpectBreakOnLine(0, session.mainScriptPath, session));
40+
}
41+
42+
function testWaitsForFrontendDisconnect(session, harness) {
43+
console.log('[test]', 'Verify node waits for the frontend to disconnect');
44+
session.sendInspectorCommands({ 'method': 'Debugger.resume'})
45+
.expectStderrOutput('Waiting for the debugger to disconnect...')
46+
.disconnect(true);
47+
}
48+
49+
function runTests(harness) {
50+
harness
51+
.runFrontendSession([
52+
testBreakpointOnStart,
53+
testWaitsForFrontendDisconnect
54+
]).expectShutDown(55);
55+
}
56+
57+
helper.startNodeForInspectorTest(runTests, ['--inspect', '--debug-brk']);
Collapse file
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const assert = require('assert');
3+
const execFile = require('child_process').execFile;
4+
const path = require('path');
5+
6+
const common = require('../common');
7+
common.skipIfInspectorDisabled();
8+
9+
const mainScript = path.join(common.fixturesDir, 'loop.js');
10+
const expected =
11+
'`node --debug` and `node --debug-brk` are invalid. ' +
12+
'Please use `node --inspect` or `node --inspect-brk` instead.';
13+
for (const invalidArg of ['--debug-brk', '--debug']) {
14+
execFile(
15+
process.execPath,
16+
[ invalidArg, mainScript ],
17+
common.mustCall((error, stdout, stderr) => {
18+
assert.strictEqual(error.code, 9, `node ${invalidArg} should exit 9`);
19+
assert.strictEqual(stderr.includes(expected), true,
20+
`${stderr} should include '${expected}'`);
21+
})
22+
);
23+
}
Collapse file
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
const common = require('../common');
3+
common.skipIfInspectorDisabled();
4+
const assert = require('assert');
5+
const spawn = require('child_process').spawn;
6+
7+
const script = common.fixturesDir + '/empty.js';
8+
9+
function test(arg) {
10+
const child = spawn(process.execPath, ['--inspect', arg, script]);
11+
const argStr = child.spawnargs.join(' ');
12+
const fail = () => assert.fail(true, false, `'${argStr}' should not quit`);
13+
child.on('exit', fail);
14+
15+
// give node time to start up the debugger
16+
setTimeout(function() {
17+
child.removeListener('exit', fail);
18+
child.kill();
19+
}, 2000);
20+
21+
process.on('exit', function() {
22+
assert(child.killed);
23+
});
24+
}
25+
26+
test('--debug-brk');
27+
test('--debug-brk=5959');

0 commit comments

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