FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: --debug* deprecation and invalidation · nodejs/node@010f864 · GitHub

/ node Public

Commit 010f864

Browse files
refack
authored and
jasnell
committed
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

10 files changed

Lines changed: 180 additions & 36 deletions

File tree

‎lib/internal/bootstrap_node.js‎

Lines 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

‎lib/internal/process/warning.js‎

Lines 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
}

‎lib/module.js‎

Lines 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');

‎src/node.cc‎

Lines 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

‎src/node_debug_options.cc‎

Lines 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
}

‎src/node_debug_options.h‎

Lines 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
};

‎test/inspector/inspector-helper.js‎

Lines 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 {

‎test/inspector/test-inspector-debug-brk.js‎

Lines 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']);

‎test/parallel/test-inspector-invalid-args.js‎

Lines 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+
}

‎test/sequential/test-debugger-debug-brk.js‎

Lines 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)
Back | FazBrowse Home | New Git URL