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

ENETUNREACH not handled for http.request in 7.10.0 #12841

Closed
cyrus-and opened this issue May 5, 2017 · 2 comments
Closed

ENETUNREACH not handled for http.request in 7.10.0 #12841

cyrus-and opened this issue May 5, 2017 · 2 comments
Labels
confirmed-bug Issues with confirmed bugs. http Issues or PRs related to the http subsystem.

Comments

@cyrus-and
Copy link

  • Version: 7.10.0
  • Platform: Linux
  • Subsystem: http

As the title says that error is not delivered via the returned event emitter, an exception is thrown instead. Other errors are properly handled, e.g., using {port: 0} which causes ECONNREFUSED. Also the net subsystem doesn't seem to be affected when I try with net.connect using the same options.

Here is the snippet to reproduce it:

const http = require('http');
const request = http.request({host: '255.255.255.255'});
request.on('error', console.error);

Incorrect behavior

$ nvm use 7.10.0
Now using node v7.10.0
$ node issue.js
events.js:163
      throw er; // Unhandled 'error' event
      ^

Error: connect ENETUNREACH 255.255.255.255:80 - Local (0.0.0.0:0)
    at Object.exports._errnoException (util.js:1050:11)
    at exports._exceptionWithHostPort (util.js:1073:20)
    at internalConnect (net.js:889:16)
    at lookupAndConnect (net.js:977:5)
    at Socket.realConnect (net.js:945:5)
    at Agent.connect [as createConnection] (net.js:77:22)
    at Agent.createSocket (_http_agent.js:195:26)
    at Agent.addRequest (_http_agent.js:157:10)
    at new ClientRequest (_http_client.js:212:16)
    at Object.request (http.js:26:10)

Expected behavior

$ nvm use 7.9.0
Now using node v7.9.0
$ node issue.js
{ Error: connect ENETUNREACH 255.255.255.255:80 - Local (0.0.0.0:0)
    at Object.exports._errnoException (util.js:1050:11)
    at exports._exceptionWithHostPort (util.js:1073:20)
    at internalConnect (net.js:894:16)
    at net.js:980:9
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
    at Module.runMain (module.js:607:11)
    at run (bootstrap_node.js:423:7)
    at startup (bootstrap_node.js:147:9)
    at bootstrap_node.js:538:3
  code: 'ENETUNREACH',
  errno: 'ENETUNREACH',
  syscall: 'connect',
  address: '255.255.255.255',
  port: 80 }
@vsemozhetbyt vsemozhetbyt added the http Issues or PRs related to the http subsystem. label May 5, 2017
@refack refack added the net Issues and PRs related to the net subsystem. label May 5, 2017
@mscdex mscdex added confirmed-bug Issues with confirmed bugs. and removed net Issues and PRs related to the net subsystem. labels May 5, 2017
@mscdex
Copy link
Contributor

mscdex commented May 5, 2017

Works fine in v7.9.0. Looks like a regression somewhere...

@mscdex
Copy link
Contributor

mscdex commented May 5, 2017

Bisecting reveals d0b1be1 is the culprit.

FWIW this is also happening in master.

/cc @bnoordhuis

cyrus-and added a commit to cyrus-and/chrome-remote-interface that referenced this issue May 9, 2017
evanlucas added a commit to evanlucas/node that referenced this issue May 23, 2017
This test ensures that a http client request with the default agent
that has a socket that is immediately destroyed can still be caught by
adding an error event listener to the request object.

PR-URL: nodejs#12854
Fixes: nodejs#12841
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
jasnell pushed a commit that referenced this issue May 23, 2017
This reverts commit 571882c.

Removing the process.nextTick() call can prevent the consumer
from being able to catch error events.

PR-URL: #12854
Fixes: #12841
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
jasnell pushed a commit that referenced this issue May 23, 2017
This test ensures that a http client request with the default agent
that has a socket that is immediately destroyed can still be caught by
adding an error event listener to the request object.

PR-URL: #12854
Fixes: #12841
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
jasnell pushed a commit that referenced this issue May 28, 2017
This reverts commit 571882c.

Removing the process.nextTick() call can prevent the consumer
from being able to catch error events.

PR-URL: #12854
Fixes: #12841
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
jasnell pushed a commit that referenced this issue May 28, 2017
This test ensures that a http client request with the default agent
that has a socket that is immediately destroyed can still be caught by
adding an error event listener to the request object.

PR-URL: #12854
Fixes: #12841
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants