Skip to content

improve error messge in lib/dns #3891

Description

@MylesBorins

Due to backwards compatibility dns.js is reporting UV_EAI_MEMORY, UV_EAI_NODATA, and UV_EAI_NONAME errors as ENOTFOUND

This is currently labelled FIXME --> https://github.com/nodejs/node/blob/master/lib/dns.js#L17-L23

ENOTFOUND is not even a proper POSIX error!
@bnoordhuis

The change breaks a few tests

  • test-http-dns-error
  • test-net-better-error-messages-port-hostname
  • test-net-connect-immediate-finish
  • test-net-dns-error

interestingly enough every testc ase we had was for EAI_NONAME, we have no tests for EAI_MEMORY or EAI_NODATA

If people are interested I'll get started on this change, including writing extra tests and smoke testing. This should be a great candidate for citgm

Activity

  1. added
    dnsIssues and PRs related to the dns subsystem.
    semver-majorPRs that contain breaking changes and should be released in the next major version.
    on Nov 18, 2015
  2. MylesBorins commented on Nov 18, 2015

    @MylesBorins
    ContributorAuthor
  3. cjihrig commented on Nov 18, 2015

    @cjihrig
    Contributor

    I'm all for removing these types of things from the codebase. My only reservation is that we don't know how much code would break for relatively little gain.

  4. jasnell commented on Nov 18, 2015

    @jasnell
    Member

    I'm torn. Like @cjihrig I don't see a lot of value but eliminating technical debt is generally a good thing. Smoke testing this would be worthwhile.

  5. silverwind commented on Nov 22, 2015

    @silverwind
    Contributor

    We discussed DNS errors in #1843, and ideally we'd like to return real DNS error codes (NXDOMAIN, SERVFAIL, REFUSED etc, see section 2.3 of https://tools.ietf.org/html/rfc2929) at some point. It'd be a huge breaking change of course, but if we're going to do it, I'd hold of from changing these now.

  6. MylesBorins commented on Apr 21, 2016

    @MylesBorins
    ContributorAuthor

    @jasnell is this something I should try and get together for v6? Will it just create too much churn?

  7. jasnell commented on Apr 21, 2016

    @jasnell
    Member

    We should likely hold off and do this post v6. It's still not yet clear how much this will impact.

  8. silverwind commented on Apr 21, 2016

    @silverwind
    Contributor

    These are some exotic errors, I guess it would be pretty low impact, except maybe UV_EAI_NONAME. I'd say we should take the chance and fix this now.

  9. MylesBorins commented on Apr 21, 2016

    @MylesBorins
    ContributorAuthor

    @jasnell with that in mind it makes sense to aim to get this in for v7, that way if it was a terrible idea we can back it out in v8 and not be stuck with it for 3 years!

  10. silverwind commented on Apr 21, 2016

    @silverwind
    Contributor

    What's the supposed fix going to be, by the way? Remove the conditional and progagate libuv error codes the user?

  11. MylesBorins commented on Apr 21, 2016

    @MylesBorins
    ContributorAuthor

    @silverwind indeed. It would be propogated proper error codes

  12. silverwind commented on Apr 21, 2016

    @silverwind
    Contributor

    Hmm, I'm just worried about our E{SOMETHING} convention. Should UV_EAI_NONAME become ENONAME ?

  13. atomantic commented on Aug 8, 2016

    @atomantic

    I've been wresting with an ENOTFOUND error (in Node.js 4.4.7) and I'd love to see deeper into why it's failing because it fails after 2,184 successful http.get calls:

    redacteddomain.com error (had 2184 successes): { [Error: getaddrinfo ENOTFOUND redacteddomain.com redacteddomain.com:80]
      code: 'ENOTFOUND',
      errno: 'ENOTFOUND',
      syscall: 'getaddrinfo',
      hostname: 'redacteddomain.com',
      host: 'redacteddomain.com',
      port: 80 }
    

    This happens within about 30 seconds after service startup.

    Any idea how to get more info on what's really happening here?

  14. bnoordhuis commented on Aug 9, 2016

    @bnoordhuis
    Member

    @atomantic I speculate that if you attach strace -f (assuming you run linux), system calls start to fail with an EMFILE or ENFILE error code when you get ENOTFOUND.

  15. atomantic commented on Aug 9, 2016

    @atomantic

    @bnoordhuis thanks. Regardless of why it's happening, I'm going to bypass this by setting up a child process DNS watchdog and simply continue to reference the target by IP. One less thing for the http request to have to do so it should be better performance anyway and removes the potential for a cache lookup or DNS resolution mishap.

  16. jasnell commented on Mar 24, 2017

    @jasnell
    Member

    ping @MylesBorins ... what do you want to do with this one?

  17. Trott commented on Jul 30, 2017

    @Trott
    Member

    I don't think this is going to happen any time soon, although a clean CITGM run would go a long way towards making it seem more possible. I'm going to close this for now, but definitely re-open it if you're working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dnsIssues and PRs related to the dns subsystem.semver-majorPRs that contain breaking changes and should be released in the next major version.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions