diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index ed1edf7138bc7e..e7d2d94c8654c0 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -65,6 +65,20 @@ }); process.argv[0] = process.execPath; + // Handle `--debug*` deprecation and invalidation + if (process._invalidDebug) { + process.emitWarning( + '`node --debug` and `node --debug-brk` are invalid. ' + + 'Please use `node --inspect` or `node --inspect-brk` instead.', + 'DeprecationWarning', 'DEP0062', startup, true); + process.exit(9); + } else if (process._deprecatedDebugBrk) { + process.emitWarning( + '`node --inspect --debug-brk` is deprecated. ' + + 'Please use `node --inspect-brk` instead.', + 'DeprecationWarning', 'DEP0062', startup, true); + } + // There are various modes that Node can run in. The most common two // are running from a script and running the REPL - but there are a few // others like the debugger or running --eval arguments. Here we decide diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js index fa77a8f3f59ce6..033005e6ed6c89 100644 --- a/lib/internal/process/warning.js +++ b/lib/internal/process/warning.js @@ -111,7 +111,7 @@ function setupProcessWarnings() { // process.emitWarning(error) // process.emitWarning(str[, type[, code]][, ctor]) // process.emitWarning(str[, options]) - process.emitWarning = function(warning, type, code, ctor) { + process.emitWarning = function(warning, type, code, ctor, now) { const errors = lazyErrors(); var detail; if (type !== null && typeof type === 'object' && !Array.isArray(type)) { @@ -150,6 +150,7 @@ function setupProcessWarnings() { if (process.throwDeprecation) throw warning; } - process.nextTick(doEmitWarning(warning)); + if (now) process.emit('warning', warning); + else process.nextTick(doEmitWarning(warning)); }; } diff --git a/lib/module.js b/lib/module.js index 5455e68dd98b22..ae6f4ff453ccc7 100644 --- a/lib/module.js +++ b/lib/module.js @@ -537,7 +537,7 @@ Module.prototype._compile = function(content, filename) { }); var inspectorWrapper = null; - if (process._debugWaitConnect && process._eval == null) { + if (process._breakFirstLine && process._eval == null) { if (!resolvedArgv) { // we enter the repl if we're not given a filename argument. if (process.argv[1]) { @@ -549,7 +549,7 @@ Module.prototype._compile = function(content, filename) { // Set breakpoint on module start if (filename === resolvedArgv) { - delete process._debugWaitConnect; + delete process._breakFirstLine; inspectorWrapper = process.binding('inspector').callAndPauseOnStart; if (!inspectorWrapper) { const Debug = vm.runInDebugContext('Debug'); diff --git a/src/node.cc b/src/node.cc index 8972dd61353518..f2688f2ecf52d5 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3391,9 +3391,29 @@ void SetupProcessObject(Environment* env, READONLY_PROPERTY(process, "traceDeprecation", True(env->isolate())); } + // TODO(refack): move the following 4 to `node_config` + // --inspect + if (debug_options.inspector_enabled()) { + READONLY_DONT_ENUM_PROPERTY(process, + "_inspectorEnbale", True(env->isolate())); + } + // --inspect-brk if (debug_options.wait_for_connect()) { - READONLY_PROPERTY(process, "_debugWaitConnect", True(env->isolate())); + READONLY_DONT_ENUM_PROPERTY(process, + "_breakFirstLine", True(env->isolate())); + } + + // --inspect --debug-brk + if (debug_options.deprecated_invocation()) { + READONLY_DONT_ENUM_PROPERTY(process, + "_deprecatedDebugBrk", True(env->isolate())); + } + + // --debug or, --debug-brk without --inspect + if (debug_options.invalid_invocation()) { + READONLY_DONT_ENUM_PROPERTY(process, + "_invalidDebug", True(env->isolate())); } // --security-revert flags diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc index 2e8f601401cba0..3f1a2e56e89985 100644 --- a/src/node_debug_options.cc +++ b/src/node_debug_options.cc @@ -8,9 +8,7 @@ namespace node { namespace { -#if HAVE_INSPECTOR const int default_inspector_port = 9229; -#endif // HAVE_INSPECTOR inline std::string remove_brackets(const std::string& host) { if (!host.empty() && host.front() == '[' && host.back() == ']') @@ -56,14 +54,12 @@ std::pair split_host_port(const std::string& arg) { } // namespace DebugOptions::DebugOptions() : -#if HAVE_INSPECTOR inspector_enabled_(false), -#endif // HAVE_INSPECTOR - wait_connect_(false), http_enabled_(false), + deprecated_debug_(false), + break_first_line_(false), host_name_("127.0.0.1"), port_(-1) { } bool DebugOptions::ParseOption(const char* argv0, const std::string& option) { - bool enable_inspector = false; bool has_argument = false; std::string option_name; std::string argument; @@ -81,11 +77,21 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) { argument.clear(); } + // Note that --debug-port and --debug-brk in conjuction with --inspect + // work but are undocumented. + // --debug is no longer valid. + // Ref: https://github.com/nodejs/node/issues/12630 + // Ref: https://github.com/nodejs/node/pull/12949 if (option_name == "--inspect") { - enable_inspector = true; + inspector_enabled_ = true; + } else if (option_name == "--debug") { + deprecated_debug_ = true; } else if (option_name == "--inspect-brk") { - enable_inspector = true; - wait_connect_ = true; + inspector_enabled_ = true; + break_first_line_ = true; + } else if (option_name == "--debug-brk") { + break_first_line_ = true; + deprecated_debug_ = true; } else if (option_name == "--debug-port" || option_name == "--inspect-port") { if (!has_argument) { @@ -97,15 +103,14 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) { return false; } - if (enable_inspector) { -#if HAVE_INSPECTOR - inspector_enabled_ = true; -#else +#if !HAVE_INSPECTOR + if (inspector_enabled_) { fprintf(stderr, "Inspector support is not available with this Node.js build\n"); - return false; -#endif } + inspector_enabled_ = false; + return false; +#endif // argument can be specified for *any* option to specify host:port if (has_argument) { @@ -124,9 +129,7 @@ bool DebugOptions::ParseOption(const char* argv0, const std::string& option) { int DebugOptions::port() const { int port = port_; if (port < 0) { -#if HAVE_INSPECTOR port = default_inspector_port; -#endif // HAVE_INSPECTOR } return port; } diff --git a/src/node_debug_options.h b/src/node_debug_options.h index fb86060f438dfc..6fdd30384fc3e8 100644 --- a/src/node_debug_options.h +++ b/src/node_debug_options.h @@ -10,25 +10,24 @@ class DebugOptions { public: DebugOptions(); bool ParseOption(const char* argv0, const std::string& option); - bool inspector_enabled() const { -#if HAVE_INSPECTOR - return inspector_enabled_; -#else - return false; -#endif // HAVE_INSPECTOR + bool inspector_enabled() const { return inspector_enabled_; } + bool deprecated_invocation() const { + return deprecated_debug_ && + inspector_enabled_ && + break_first_line_; } - bool ToolsServerEnabled(); - bool wait_for_connect() const { return wait_connect_; } + bool invalid_invocation() const { + return deprecated_debug_ && !inspector_enabled_; + } + bool wait_for_connect() const { return break_first_line_; } std::string host_name() const { return host_name_; } int port() const; void set_port(int port) { port_ = port; } private: -#if HAVE_INSPECTOR bool inspector_enabled_; -#endif // HAVE_INSPECTOR - bool wait_connect_; - bool http_enabled_; + bool deprecated_debug_; + bool break_first_line_; std::string host_name_; int port_; }; diff --git a/test/inspector/inspector-helper.js b/test/inspector/inspector-helper.js index 823064fce8a9b3..755c357077752d 100644 --- a/test/inspector/inspector-helper.js +++ b/test/inspector/inspector-helper.js @@ -496,9 +496,9 @@ Harness.prototype.kill = function() { }; exports.startNodeForInspectorTest = function(callback, - inspectorFlag = '--inspect-brk', + inspectorFlags = ['--inspect-brk'], opt_script_contents) { - const args = [inspectorFlag]; + const args = [].concat(inspectorFlags); if (opt_script_contents) { args.push('-e', opt_script_contents); } else { diff --git a/test/inspector/test-inspector-debug-brk.js b/test/inspector/test-inspector-debug-brk.js new file mode 100644 index 00000000000000..a5cb77250de439 --- /dev/null +++ b/test/inspector/test-inspector-debug-brk.js @@ -0,0 +1,57 @@ +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); +const assert = require('assert'); +const helper = require('./inspector-helper.js'); + +function setupExpectBreakOnLine(line, url, session, scopeIdCallback) { + return function(message) { + if ('Debugger.paused' === message['method']) { + const callFrame = message['params']['callFrames'][0]; + const location = callFrame['location']; + assert.strictEqual(url, session.scriptUrlForId(location['scriptId'])); + assert.strictEqual(line, location['lineNumber']); + scopeIdCallback && + scopeIdCallback(callFrame['scopeChain'][0]['object']['objectId']); + return true; + } + }; +} + +function testBreakpointOnStart(session) { + const commands = [ + { 'method': 'Runtime.enable' }, + { 'method': 'Debugger.enable' }, + { 'method': 'Debugger.setPauseOnExceptions', + 'params': {'state': 'none'} }, + { 'method': 'Debugger.setAsyncCallStackDepth', + 'params': {'maxDepth': 0} }, + { 'method': 'Profiler.enable' }, + { 'method': 'Profiler.setSamplingInterval', + 'params': {'interval': 100} }, + { 'method': 'Debugger.setBlackboxPatterns', + 'params': {'patterns': []} }, + { 'method': 'Runtime.runIfWaitingForDebugger' } + ]; + + session + .sendInspectorCommands(commands) + .expectMessages(setupExpectBreakOnLine(0, session.mainScriptPath, session)); +} + +function testWaitsForFrontendDisconnect(session, harness) { + console.log('[test]', 'Verify node waits for the frontend to disconnect'); + session.sendInspectorCommands({ 'method': 'Debugger.resume'}) + .expectStderrOutput('Waiting for the debugger to disconnect...') + .disconnect(true); +} + +function runTests(harness) { + harness + .runFrontendSession([ + testBreakpointOnStart, + testWaitsForFrontendDisconnect + ]).expectShutDown(55); +} + +helper.startNodeForInspectorTest(runTests, ['--inspect', '--debug-brk']); diff --git a/test/parallel/test-inspector-invalid-args.js b/test/parallel/test-inspector-invalid-args.js new file mode 100644 index 00000000000000..6e249470230abb --- /dev/null +++ b/test/parallel/test-inspector-invalid-args.js @@ -0,0 +1,23 @@ +'use strict'; +const assert = require('assert'); +const execFile = require('child_process').execFile; +const path = require('path'); + +const common = require('../common'); +common.skipIfInspectorDisabled(); + +const mainScript = path.join(common.fixturesDir, 'loop.js'); +const expected = + '`node --debug` and `node --debug-brk` are invalid. ' + + 'Please use `node --inspect` or `node --inspect-brk` instead.'; +for (const invalidArg of ['--debug-brk', '--debug']) { + execFile( + process.execPath, + [ invalidArg, mainScript ], + common.mustCall((error, stdout, stderr) => { + assert.strictEqual(error.code, 9, `node ${invalidArg} should exit 9`); + assert.strictEqual(stderr.includes(expected), true, + `${stderr} should include '${expected}'`); + }) + ); +} diff --git a/test/sequential/test-debugger-debug-brk.js b/test/sequential/test-debugger-debug-brk.js new file mode 100644 index 00000000000000..f6a7ce0605ad07 --- /dev/null +++ b/test/sequential/test-debugger-debug-brk.js @@ -0,0 +1,27 @@ +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); +const assert = require('assert'); +const spawn = require('child_process').spawn; + +const script = common.fixturesDir + '/empty.js'; + +function test(arg) { + const child = spawn(process.execPath, ['--inspect', arg, script]); + const argStr = child.spawnargs.join(' '); + const fail = () => assert.fail(true, false, `'${argStr}' should not quit`); + child.on('exit', fail); + + // give node time to start up the debugger + setTimeout(function() { + child.removeListener('exit', fail); + child.kill(); + }, 2000); + + process.on('exit', function() { + assert(child.killed); + }); +} + +test('--debug-brk'); +test('--debug-brk=5959');