Skip to content

Commit

Permalink
inspector: document bad usage for --inspect-port
Browse files Browse the repository at this point in the history
Document --inspect-port, and fix the reporting for when it is misused.

The option requires an argument, but when the argument was omitted, the
error message incorrectly reported --inspect-port as being bad, as if
was not supported at all:

    % node --inspect-port
    node: bad option: --inspect-port
    % node --none-such
    node: bad option: --none-such

It is now correctly reported as requiring an argument:

    % ./node --inspect-port
    ./node: --inspect-port requires an argument

PR-URL: #12581
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
sam-github authored and jasnell committed May 28, 2017
1 parent d4e9e0f commit 39785c7
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 9 deletions.
12 changes: 12 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,18 @@ added: v7.6.0
-->

Activate inspector on host:port and break at start of user script.
Default host:port is 127.0.0.1:9229.


### `--inspect-port=[host:]port`
<!-- YAML
added: v7.6.0
-->

Set the host:port to be used when the inspector is activated.
Useful when activating the inspector by sending the `SIGUSR1` signal.

Default host is 127.0.0.1.


### `--no-deprecation`
Expand Down
4 changes: 4 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ instances for debugging and profiling. It uses the Chrome Debugging Protocol.
.BR \-\-inspect-brk \fI[=[host:]port]\fR
Activate inspector on host:port and break at start of user script.

.TP
.BR \-\-inspect-port \fI=[host:]port\fR
Set the host:port to be used when the inspector is activated.

.TP
.BR \-\-no\-deprecation
Silence deprecation warnings.
Expand Down
4 changes: 3 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3612,6 +3612,8 @@ static void PrintHelp() {
" --inspect-brk[=[host:]port]\n"
" activate inspector on host:port\n"
" and break at start of user script\n"
" --inspect-port=[host:]port\n"
" set host:port for inspector\n"
#endif
" --no-deprecation silence deprecation warnings\n"
" --trace-deprecation show stack traces on deprecations\n"
Expand Down Expand Up @@ -3798,7 +3800,7 @@ static void ParseArgs(int* argc,

CheckIfAllowedInEnv(argv[0], is_env, arg);

if (debug_options.ParseOption(arg)) {
if (debug_options.ParseOption(argv[0], arg)) {
// Done, consumed by DebugOptions::ParseOption().
} else if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) {
printf("%s\n", NODE_VERSION);
Expand Down
21 changes: 14 additions & 7 deletions src/node_debug_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ DebugOptions::DebugOptions() :
wait_connect_(false), http_enabled_(false),
host_name_("127.0.0.1"), port_(-1) { }

bool DebugOptions::ParseOption(const std::string& option) {
bool DebugOptions::ParseOption(const char* argv0, const std::string& option) {
bool enable_inspector = false;
bool has_argument = false;
std::string option_name;
Expand All @@ -72,21 +72,28 @@ bool DebugOptions::ParseOption(const std::string& option) {
if (pos == std::string::npos) {
option_name = option;
} else {
has_argument = true;
option_name = option.substr(0, pos);
argument = option.substr(pos + 1);

if (argument.length() > 0)
has_argument = true;
else
argument.clear();
}

if (option_name == "--inspect") {
enable_inspector = true;
} else if (option_name == "--inspect-brk") {
enable_inspector = true;
wait_connect_ = true;
} else if ((option_name != "--debug-port" &&
option_name != "--inspect-port") ||
!has_argument) {
// only other valid possibility is --inspect-port,
// which requires an argument
} else if (option_name == "--debug-port" ||
option_name == "--inspect-port") {
if (!has_argument) {
fprintf(stderr, "%s: %s requires an argument\n",
argv0, option.c_str());
exit(9);
}
} else {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/node_debug_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace node {
class DebugOptions {
public:
DebugOptions();
bool ParseOption(const std::string& option);
bool ParseOption(const char* argv0, const std::string& option);
bool inspector_enabled() const {
#if HAVE_INSPECTOR
return inspector_enabled_;
Expand Down
23 changes: 23 additions & 0 deletions test/parallel/test-cli-bad-options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
require('../common');

// Tests that node exits consistently on bad option syntax.

const assert = require('assert');
const spawn = require('child_process').spawnSync;

requiresArgument('--inspect-port');
requiresArgument('--inspect-port=');
requiresArgument('--debug-port');
requiresArgument('--debug-port=');
requiresArgument('--eval');

function requiresArgument(option) {
const r = spawn(process.execPath, [option], {encoding: 'utf8'});

assert.strictEqual(r.status, 9);

const msg = r.stderr.split(/\r?\n/)[0];
assert.strictEqual(msg, process.execPath + ': ' + option +
' requires an argument');
}

0 comments on commit 39785c7

Please sign in to comment.