-
Notifications
You must be signed in to change notification settings - Fork 28.8k
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
debugger: don't spawn child process in remote mode #1282
Conversation
cc @bnoordhuis , would you like to review this patch? this pull request fix issue #889 . |
if (connectionAttempts >= 10) { | ||
client.removeListener('error', connectError); | ||
self.stdout.write(' failed, please retry\n'); | ||
return; | ||
} | ||
setTimeout(attemptConnect, 500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo? attemptConnect is called connect now, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not typo. attemptConnect
is called, but failed.
94d3bab
to
88267eb
Compare
ping @bnoordhuis , I have updated by you comments. |
LGTM but would it be possible to add a regression test for #889? |
When debug in remote mode with host:port or pid, the interface spawn child process also. If the debugger agent is running, will get following output: ``` < Error: listen EADDRINUSE :::5858 < at Object.exports._errnoException (util.js:734:11) < at exports._exceptionWithHostPort (util.js:757:20) < at Agent.Server._listen2 (net.js:1155:14) < at listen (net.js:1181:10) < at Agent.Server.listen (net.js:1268:5) < at Object.start (_debug_agent.js:21:9) < at startup (node.js:68:9) < at node.js:799:3 ``` This fix won't spawn child process and no more error message was shown. When use `iojs debug`, the tip information just like this: ``` Usage: iojs debug script.js ``` This fix will display the advance usage also: ``` Usage: iojs debug script.js iojs debug <host>:<port> iojs debug -p <pid> ```
88267eb
to
e4180be
Compare
@bnoordhuis done. |
When debug in remote mode with host:port or pid, the interface spawn child process also. If the debugger agent is running, will get following output: ``` < Error: listen EADDRINUSE :::5858 < at Object.exports._errnoException (util.js:734:11) < at exports._exceptionWithHostPort (util.js:757:20) < at Agent.Server._listen2 (net.js:1155:14) < at listen (net.js:1181:10) < at Agent.Server.listen (net.js:1268:5) < at Object.start (_debug_agent.js:21:9) < at startup (node.js:68:9) < at node.js:799:3 ``` This fix won't spawn child process and no more error message was shown. When use `iojs debug`, the tip information just like this: ``` Usage: iojs debug script.js ``` This fix will display the advance usage also: ``` Usage: iojs debug script.js iojs debug <host>:<port> iojs debug -p <pid> ``` Fixes: #889 PR-URL: #1282 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Excellent, thanks. Landed in a2ea168. I changed the test a little to use process.execPath instead of ./node. Thanks again. |
We still experience the issue angular/protractor#2039 |
@melnikaite Can you file a new issue and include a small test case or steps to reproduce? |
When debug in remote mode with host:port or pid, the interface
spawn child process also. If the debugger agent is running, will
get following output:
This fix won't spawn child process and no more error message was
shown.
When use
iojs debug
, the tip information just like this:This fix will display the advance usage also: