Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

better network error messages #7005

Closed
tj opened this issue Jan 30, 2014 · 14 comments
Closed

better network error messages #7005

tj opened this issue Jan 30, 2014 · 14 comments

Comments

@tj
Copy link

tj commented Jan 30, 2014

I've brought this up before but I can't find the issue, or if/why it was denied, but anyway I think we could do a lot better with network related error reporting:

Error: connect ECONNREFUSED
    at errnoException (net.js:901:11)
    at Object.afterConnect [as oncomplete] (net.js:892:19)

isn't overly informative, it would be awesome if (when available) we tack on some information, host/port etc. It's easy to argue that when you receive the error you should be printing it or doing something with it in a way that makes the message more useful, but it certainly wouldn't hurt for cases where you just use a console.error(err.stack) etc

@tjfontaine
Copy link

That's a reasonable request, I would enthusiastically review such a PR

@trevnorris
Copy link

Don't have source in front of me, but do we make sure to always attach the
E* to error.code?

@tjfontaine
Copy link

The code is already supplied in the example, econnreset, but like we
do for child process of including command and args, for network errors
we could provide context like remote and local host and port

@tj
Copy link
Author

tj commented Jan 31, 2014

rad! I'll try and send a patch over in the next few days when I have a chance

@bpytlik
Copy link

bpytlik commented Feb 4, 2014

Since issue #7042 was closed as a dup of this one, please take a broad interpretation of this issue to also cover things like failed dns lookup. The lack of information is a broader problem than just net.js. It's just my 2 cents, but it might be worth having Socket.prototype.send (in dgram.js) handle a bunch of these issues. I don't know if your path to net.js also passes through that code, but in my case that seemed like the place that really had the useful information about what was being attempted.

@tj
Copy link
Author

tj commented Mar 8, 2014

haven't had time for a patch yet but yeah this is a massive issue in production haha :D. Nothing worse than getting this in your logs, IMO this should be pretty high priority, it renders node pretty useless unless you manually go tag every single error in the system

  { message: 'write EPIPE',
    stack: 'Error: write EPIPE\n    at errnoException (net.js:770:11)\n    at Object.afterWrite (net.js:594:19)',
    code: 'EPIPE',
    errno: 'EPIPE',
    syscall: 'write',
    app: 'ingestion' }

EDIT:

something like:

{ message: '10.0.1.99:1234 - write EPIPE',
  host: '10.0.1.99',
  port: 1234
...

@rauchg
Copy link

rauchg commented Mar 8, 2014

+1 – we've gone extremely long with these uninformative errors. Definitely nice for 0.12

@Fishrock123
Copy link

👍

@jpillora
Copy link

jpillora commented Mar 9, 2014

👍 (anyone else get double comments when posting from mobile?)

@matejkramny
Copy link

👍

1 similar comment
@fyockm
Copy link

fyockm commented Mar 10, 2014

👍

evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Mar 10, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Mar 10, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Mar 13, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Mar 21, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Apr 11, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Jun 5, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Jul 13, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas added a commit to evanlucas/node-v0.x-archive that referenced this issue Aug 13, 2014
Add address and/or port to errors where applicable for better reporting.

See nodejs#7005

Use util.format instead of string concatenation

Also move some logic into util._errnoException. Passing an object as
the fourth parameter to util._errnoException will now extend the
returned error.
evanlucas pushed a commit to evanlucas/node that referenced this issue Dec 6, 2014
Add address and/or port to errors where applicable for better reporting.
In the event the local address and port are accessible, it will also add
those to the error message.

See nodejs/node-v0.x-archive#7005
indutny pushed a commit to nodejs/node that referenced this issue Dec 6, 2014
Add address and/or port to errors where applicable for better reporting.
In the event the local address and port are accessible, it will also add
those to the error message.

See nodejs/node-v0.x-archive#7005
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: #35
@jasnell
Copy link
Member

jasnell commented May 28, 2015

@joyent/node-coreteam ... do we want to keep this open here? error message improvements were made in io.js and will come along in the converged stream. It's possible we may want to backport many of those fixes, and it's possible that many more improvements could still be made.

@trevnorris
Copy link

I'd just wait until they're converged before backporting.

@jasnell
Copy link
Member

jasnell commented Jun 1, 2015

Agreed. Closing. Can reopen if folks feel it's worthwhile to backport those changes.

@jasnell jasnell closed this as completed Jun 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants