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

Improve error message for address already in use #22936

Closed
ericandrewlewis opened this issue Sep 18, 2018 · 10 comments
Closed

Improve error message for address already in use #22936

ericandrewlewis opened this issue Sep 18, 2018 · 10 comments
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. net Issues and PRs related to the net subsystem.

Comments

@ericandrewlewis
Copy link

Hello! This is my first issue on this repository, thanks for Node and all that y'all do!

In my class, sometimes students will start an Express server on the same port of one running in another terminal tab.

They get an error message like this:

15:20 $ node server.js 
events.js:167
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE :::5678
    at Server.setupListenHandle [as _listen2] (net.js:1336:14)
    at listenInCluster (net.js:1384:12)
    at Server.listen (net.js:1471:7)
    at Function.listen (/Users/ericlewis/Desktop/js-express-and-sequelize-lab/node_modules/express/lib/application.js:618:24)
    at Object.<anonymous> (/Users/ericlewis/Desktop/js-express-and-sequelize-lab/server.js:11:5)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
Emitted 'error' event at:
    at emitErrorNT (net.js:1363:8)
    at process._tickCallback (internal/process/next_tick.js:63:19)
    at Function.Module.runMain (internal/modules/cjs/loader.js:745:11)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)

I wonder if we could improve this error message because it's a bit ambiguous what the actual problem is to newcomers.

I saw that this already came up in the Express issue tracker, where the suggestion was to change the error message in Node itself. The suggestion there was to add this context:

EADDRINUSE: http://localhost:3000 is already in use
@mscdex
Copy link
Contributor

mscdex commented Sep 18, 2018

'::' is not localhost though, it's the ipv6 equivalent of '0.0.0.0'.

Also, I don't think putting protocol-specific information like 'http' in the error message is worthwhile. FWIW you can attach an 'error' event handler to your http server before calling listen() (explicitly or not) and get all of the information you need (errno/code, syscall, address, port) from the error object if you want to provide your own custom formatted message.

@ericandrewlewis
Copy link
Author

'::' is not localhost though, it's the ipv6 equivalent of '0.0.0.0'.
Also, I don't think putting protocol-specific information like 'http' in the error message is worthwhile.

Good point! What do you think of a more generalized message like

Cannot listen on :::3000. Another server on the local system is occupying that address.

@joyeecheung
Copy link
Member

Before changing the format of the message, we may need to deprecate/remove util._exceptionWithHostPort() first.

@joyeecheung joyeecheung added net Issues and PRs related to the net subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Sep 19, 2018
@sagitsofan
Copy link
Contributor

sagitsofan commented Sep 19, 2018

What is the reason for deprecate exceptionWithHostPort function inside the error.js file ?

@joyeecheung
Copy link
Member

joyeecheung commented Sep 19, 2018

@sagitsofan The error messages look like this because the error goes through exceptionWithHostPort so to change it you'll need to either modify that function (which is exported by accident) or keep the exported one and create a new internal function (introducing difference between the two).

IMO modifying or removing util._exceptionWithHostPort would be a breaking change.

@joyeecheung
Copy link
Member

joyeecheung commented Sep 19, 2018

On EADDRINUSE in particular: the human-readable error message defined by libuv is address already in use. If we are going to make the error message more readable, it would be easier to translate the code to what libuv defines (basically means doing something similar to what uvException in internal/errors does), than trying to rewrite human-readable error messages for every one of the 70+ types of errors.

@sagitsofan
Copy link
Contributor

sagitsofan commented Sep 20, 2018

Will send a new PR soon

@sagitsofan
Copy link
Contributor

PR #22995

@sagitsofan
Copy link
Contributor

sagitsofan commented Sep 28, 2018

@joyeecheung

@sagitsofan
Copy link
Contributor

ping

sagitsofan added a commit to sagitsofan/node that referenced this issue Oct 12, 2018
…rrors.

currently changed only in `setupListenHandle`, but needs to be change all over.

Added new `uvExceptionWithHostPort` function (+export) in `lib/internal/error.js` that extracts the error message defined by libuv, using the error code, and returns an error object with the full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from `lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function `uvExceptionWithHostPort` for a more detailed error.

Fixes: nodejs#22936

<!--
Thank you for your pull request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests and possibly benchmarks.

Contributors guide: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md
-->

<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [X] `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes
- [X] commit message follows [commit guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines)
sagitsofan added a commit to sagitsofan/node that referenced this issue Oct 12, 2018
Comments clarification at the deprecated function and new function.

Fixes: nodejs#22936
sagitsofan added a commit to sagitsofan/node that referenced this issue Oct 12, 2018
Comments clarification

Fixes: nodejs#22936
Trott pushed a commit that referenced this issue Oct 13, 2018
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: #22936

PR-URL: #22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Oct 13, 2018
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: #22936

PR-URL: #22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this issue Oct 17, 2018
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: #22936

PR-URL: #22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Oct 30, 2018
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: #22936

PR-URL: #22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this issue Nov 28, 2018
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: #22936

PR-URL: #22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 29, 2018
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: #22936

PR-URL: #22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants