inspector: print all listening addresses - #26008
Conversation
Sorry, something went wrong.
Some hostnames have multiple interfaces. Before this commit, the inspector only printed the first one. Now, it prints them all. No test. I can't think of a reliable way to test this on the CI matrix. Fixes: nodejs#13772
|
@bnoordhuis build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2548/pipeline |
Sorry, something went wrong.
|
Locally: D:\code\4node\gyp>d:\refael\Downloads\node.exe --inspect=localhost:0 Debugger listening on ws://localhost:56121/d4d038da-ff6a-4e0b-ac0c-d5cad82c928a Debugger listening on ws://localhost:56122/d4d038da-ff6a-4e0b-ac0c-d5cad82c928a For help, see: https://nodejs.org/en/docs/inspector |
Sorry, something went wrong.
refack
left a comment
There was a problem hiding this comment.
test failures possible, some tests probably assume the current output. Everything passes for me locally though.
Since nothing failed, we probably should have a test for this.
Sorry, something went wrong.
Yes, but how? CI machines may or may not be multi-homed, kind of difficult to test for. I could write a test that checks with dns.lookup() if a hostname (and: what hostname?) resolves to more than one address but it would be dead code if no CI machine is actually multi-homed. |
Sorry, something went wrong.
|
The one failure seems to be infrastructural? 08:50:58 + docker exec node-ci-jessie /bin/sh -c 'cd /home/iojs/build/workspace/node-test-binary-arm && . /home/iojs/build/workspace/node-test-binary-arm/node-ci-exec' 08:51:00 OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "process_linux.go:90: adding pid 26098 to cgroups caused \"failed to write 26098 to cgroup.procs: write /sys/fs/cgroup/cpu,cpuacct/docker/a4e4fbb3c89a89856f299285777bc58d01f4a0275208f0ff49a4db562fadbf55/cgroup.procs: invalid argument\"": unknown |
Sorry, something went wrong.
I think that if a machine is both IPv4 & IPv6 enabled in will assign two ports (well at least for windows it will)... And we have those predicates in test/common. Worth a shot. |
Sorry, something went wrong.
If you're 100% positive it works that way for Windows I could write that test. On Unices, localhost is only going to be mapped to two addresses when it has two entries in /etc/hosts but that's uncommon. Most distros call it something like ip6-localhost. |
Sorry, something went wrong.
eugeneo
left a comment
There was a problem hiding this comment.
I just want to point out that this was not implemented originally because of concerns that some clients parsing the output might be broken due to unexpected second and such message. I strongly believe such clients should be fixed :)
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Some hostnames have multiple interfaces. Before this commit, the inspector only printed the first one. Now, it prints them all. No test. I can't think of a reliable way to test this on the CI matrix. PR-URL: nodejs#26008 Fixes: nodejs#13772 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Some hostnames have multiple interfaces. Before this commit, the inspector only printed the first one. Now, it prints them all. No test. I can't think of a reliable way to test this on the CI matrix. PR-URL: #26008 Fixes: #13772 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Some hostnames have multiple interfaces. Before this commit, the inspector only printed the first one. Now, it prints them all. No test. I can't think of a reliable way to test this on the CI matrix. PR-URL: #26008 Fixes: #13772 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Some hostnames have multiple interfaces. Before this commit, the inspector only printed the first one. Now, it prints them all. No test. I can't think of a reliable way to test this on the CI matrix. PR-URL: #26008 Fixes: #13772 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Some hostnames have multiple interfaces. Before this commit, the
inspector only printed the first one. Now, it prints them all.
No test. I can't think of a reliable way to test this on the CI matrix.
Fixes: #13772
CI: https://ci.nodejs.org/job/node-test-pull-request/20671/ - test failures possible, some tests probably assume the current output. Everything passes for me locally though.