Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster fails with NODE_OPTIONS="--inspect" #19026

Closed
mcfedr opened this issue Feb 27, 2018 · 2 comments
Closed

Cluster fails with NODE_OPTIONS="--inspect" #19026

mcfedr opened this issue Feb 27, 2018 · 2 comments
Labels
inspector Issues and PRs related to the V8 inspector protocol

Comments

@mcfedr
Copy link

mcfedr commented Feb 27, 2018

  • Version: v9.6.1
  • Platform: MacOS
  • Subsystem: Cluster

When using cluster and --inspect as cli argument it is correctly handled and each worker will use a different port, this was fixed recently by #13619

But when env var NODE_OPTIONS="--inspect" is set this logic doesn't apply and the workers will fail as they try to attach to the same port

Working:

alexander ~/dev/cluster-test $ node --inspect index.js
Debugger listening on ws://127.0.0.1:9229/e6bf8c7c-bf59-4f65-a924-ab050d5a41f3
For help see https://nodejs.org/en/docs/inspector
Debugger listening on ws://127.0.0.1:9230/a32c88e4-5e0a-4140-a648-8323d0758c15
For help see https://nodejs.org/en/docs/inspector
worker success!

Failed:

alexander ~/dev/cluster-test $ NODE_OPTIONS="--inspect" node index.js
Debugger listening on ws://127.0.0.1:9229/57f0f9f7-272e-459f-94a7-b4b48c676d9d
For help see https://nodejs.org/en/docs/inspector
Starting inspector on 127.0.0.1:9229 failed: address already in use
worker exited with error code: 12
@gibfahn gibfahn added the inspector Issues and PRs related to the V8 inspector protocol label Feb 27, 2018
@richardlau
Copy link
Member

I think amending the condition

if (execArgv.some((arg) => arg.match(debugArgRegex))) {
to additionally match the regexp to NODE_OPTIONS (if it exists) would fix this.

Would also need tests (which is the reason I haven't raised a quick PR). If no-one else picks this up I'll try to find some time next week. (If anyone does feel like picking this up, just raise a PR -- no need to ask permission first 😄 ).

@sameer-coder
Copy link
Contributor

I have been working on this issue and have created a PR (#19165).
Added tests as well.
@maintainers, Please review the code.

This is my first PR and I have tried to make sure that I follow all the guide lines. Please let me know if I have missed something or if any changes are required.

sameer-coder added a commit to sameer-coder/node that referenced this issue Mar 16, 2018
When using cluster and --inspect as cli argument it is correctly
handled and each worker will use a different port, this was
fixed by nodejs#13619. But when env var NODE_OPTIONS="--inspect"
is set this logic doesn't apply and the workers will fail as they
try to attach to the same port.

Fixes: nodejs#19026
sameer-coder added a commit to sameer-coder/node that referenced this issue Mar 16, 2018
…k with NODE_OPTIONS="--inspect"

    When using cluster and --inspect as cli argument it is correctly
    handled and each worker will use a different port, this was
    fixed by nodejs#13619. But when env var NODE_OPTIONS="--inspect"
    is set this logic doesn't apply and the workers will fail as they
    try to attach to the same port.

    Fixes: nodejs#19026
sameer-coder added a commit to sameer-coder/node that referenced this issue Mar 16, 2018
…or-node_options

Changed multi-line template string in test-inspect-support-for-node_options
as per feedback from @jasnell. PR nodejs#19165

Fixes : nodejs#19026
sameer-coder added a commit to sameer-coder/node that referenced this issue Mar 16, 2018
…ptions

Removed numCpus and hardcoded to spawn multiple workers in test-inspect-support-for-node_options.
PR nodejs#19165

Fixes : nodejs#19026
sameer-coder added a commit to sameer-coder/node that referenced this issue Mar 16, 2018
…rt-for-node_options

Using common.mustCall to wrap callback and added assert to replace if-fail
in test-inspect-support-for-node_options. PR nodejs#19165

Fixes : nodejs#19026
sameer-coder added a commit to sameer-coder/node that referenced this issue Mar 17, 2018
const numWorkers added to specify number of workers spawned
in test-inspect-support-for-node_options. PR nodejs#19165

Fixes : nodejs#19026
targos pushed a commit that referenced this issue Mar 24, 2018
When using cluster and --inspect as cli argument it is correctly
handled and each worker will use a different port, this was
fixed by #13619. But when env var NODE_OPTIONS="--inspect"
is set this logic doesn't apply and the workers will fail as they
try to attach to the same port.

Fixes: #19026

PR-URL: #19165
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants