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

ENOTFOUND is a real error; lets document it! #26484

Closed
bterlson opened this issue Mar 7, 2019 · 4 comments
Closed

ENOTFOUND is a real error; lets document it! #26484

bterlson opened this issue Mar 7, 2019 · 4 comments

Comments

@bterlson
Copy link

bterlson commented Mar 7, 2019

  • Version: 11.x
  • Platform: Any
  • Subsystem: Net

As best as I can tell, network failure often surfaces to the Node user with an error ENOTFOUND. In attempting to determine under what conditions this error is thrown, we found that the ENOTFOUND error is not documented.

@addaleax pointed out that there's a FIXME comment from @bnoordhuis suggesting this error is temporary, but I bet ENOTFOUND is pretty baked and changing it would cause significant friction.

So I'd like to propose the following:

  • Remove the FIXME comment
  • Document ENOTFOUND under "Common System Errors". It's not an actual system error but it appears like one so users might expect to find it there anyway.
  • @Fishrock123 also suggested putting the actual error code on the error object somewhere, which would be very handy.
@BridgeAR
Copy link
Member

BridgeAR commented Mar 7, 2019

It indeed seems to late to fix that inconsistency. Adding the actual error code to the error also seems like a good idea. The only question for me is how the property should be named.

We might also want to check if let's say 90% of all common network errors are of either of the two types. If that is the case, we might want to just use ENOTFOUND as alias to the actual one that makes up for the 90% and return the original error code that makes up for the 10%. That way we could at least reduce the inconsistency a bit without breaking much. It might just not be trivial to determine how common each of those two is.

@addaleax
Copy link
Member

addaleax commented Mar 7, 2019

I think in practice applications wouldn’t make a difference between UV_EAI_NODATA and UV_EAI_NONAME, given that the effect is ultimately the same – the address could not be resolved.

So I don’t think aliasing one of these errors to ENOTFOUD is necessary.

@BridgeAR
Copy link
Member

BridgeAR commented Mar 7, 2019

I agree that it likely makes little difference for the user. That is why I'd like to add an extra property (e.g., GROUP) that contains 'NETWORK_ERR' (or similar) as value for all network errors as most users would not care about the specific error.

cjihrig added a commit to cjihrig/node that referenced this issue Mar 10, 2019
PR-URL: nodejs#26495
Fixes: nodejs#26484
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@bterlson
Copy link
Author

@cjihrig and reviewers, thank you so much!

BridgeAR pushed a commit that referenced this issue Mar 13, 2019
PR-URL: #26495
Fixes: #26484
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR pushed a commit that referenced this issue Mar 13, 2019
PR-URL: #26495
Fixes: #26484
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR pushed a commit that referenced this issue Mar 14, 2019
PR-URL: #26495
Fixes: #26484
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants