Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

fix: Make --inspect-port=0 work #51

Merged
merged 1 commit into from
Oct 30, 2017
Merged

fix: Make --inspect-port=0 work #51

merged 1 commit into from
Oct 30, 2017

Conversation

bnoordhuis
Copy link
Member

Selecting port zero makes the inspector bind to a random port.

The inspector prints the URL it listens on to stderr. Parse the
output from the child process to find the port number.

This commit coincidentally also makes node inspect --port=0 x.js
work and a regression test to that effect has been added.

Note that connecting to an existing process that listens on a
non-standard port with node inspect -p <pid> does not work
because then we never see the output from the inspector.

Fixes: nodejs/node#16469

Selecting port zero makes the inspector bind to a random port.

The inspector prints the URL it listens on to stderr.  Parse the
output from the child process to find the port number.

This commit coincidentally also makes `node inspect --port=0 x.js`
work and a regression test to that effect has been added.

Note that connecting to an existing process that listens on a
non-standard port with `node inspect -p <pid>` does not work
because then we never see the output from the inspector.

Fixes: nodejs/node#16469
Copy link
Collaborator

@jkrems jkrems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jkrems
Copy link
Collaborator

jkrems commented Oct 27, 2017

Triggering CI and hoping there's no unconnected failure in recent node releases... https://ci.nodejs.org/view/x%20-%20Diagnostics/job/node-inspect-continuous-integration/41/

@jkrems
Copy link
Collaborator

jkrems commented Oct 27, 2017

Triggering a master build to make sure the Windows failures are a general problem. Don't think that's connected to this PR.

EDIT: Looks like a problem with the CI configuration for the Windows builds. Going to merge this change and try to figure out how to fix it independently. Maybe this is a good opportunity to invest into writing that Jenkinsfile.

@jkrems jkrems merged commit 49d7f88 into nodejs:master Oct 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants