You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add version of dns.lookup that returns all results #736
The dns module provides two sets of methods: the resolve family, which directly resolves using the DNS protocol, and the lookup function, which uses the system resolver (through getaddrinfo) and returns at most one address.
It's often preferable to use the system resolver, since it can use things like mDNS. However, using the dns.lookup API makes it impossible to receive multiple IP addresses in response to a DNS query (See #708 for motivating example).
Proposal: Add a new function dns.lookupAll that behaves like dns.lookup, but returns all addresses given by getaddrinfo.
This seems like a pretty simple addition. I'd be fine with working on it, if others find it useful. I think the window is currently open for a minor version bump too.
This seems like a perfectly reasonable addition to me. It doesn't even need a new function; dns.lookup() already looks up all the addresses but it's intentionally crippled to only report the first one. We can simply extend it like this:
The callback gets the address family as its third argument now. For the { all: true } case, we probably want to drop that and report the family in the addrs array. It's a list of strings now, that would have to change to a list of objects.
@silverwind sure. I was thinking you might just want to change this line to point to another function. And the callback would change from function(err, address, family) to function(err, addresses), where each element in addresses would be something like {ip: '127.0.0.1', family: 4}.
My first approach would've been to modify onlookup with additional logic and add a req.all property. You think that a second function would be cleaner?
I figured it would be simpler, with less chance of breaking existing code, to just point to a new function. The two functions will have very little overlap except checking for an error.
Any opinions if the address field in the object should be called address or ip? I'm leaning more towards address for correctness, but @cjihrig suggested ip. Also, the docs usually call it address.
I'll put this up shortly for a PR, just have to test on OS X, as apparently my Windows machine isn't able to dns.lookup for an IPv6 (getting ENOENT), but dns.resolve6 strangely works. This is on v1.x too. Might be a separate issue.
Activity
cjihrig commentedon Feb 6, 2015
This seems like a pretty simple addition. I'd be fine with working on it, if others find it useful. I think the window is currently open for a minor version bump too.
bnoordhuis commentedon Feb 6, 2015
This seems like a perfectly reasonable addition to me. It doesn't even need a new function;
dns.lookup()already looks up all the addresses but it's intentionally crippled to only report the first one. We can simply extend it like this:The callback gets the address family as its third argument now. For the
{ all: true }case, we probably want to drop that and report the family in theaddrsarray. It's a list of strings now, that would have to change to a list of objects.silverwind commentedon Feb 6, 2015
I'll start working on this according to @bnoordhuis's proposal, if that's ok with you, @cjihrig.
cjihrig commentedon Feb 6, 2015
@silverwind sure. I was thinking you might just want to change this line to point to another function. And the callback would change from
function(err, address, family)tofunction(err, addresses), where each element inaddresseswould be something like{ip: '127.0.0.1', family: 4}.silverwind commentedon Feb 6, 2015
My first approach would've been to modify
onlookupwith additional logic and add areq.allproperty. You think that a second function would be cleaner?cjihrig commentedon Feb 6, 2015
I figured it would be simpler, with less chance of breaking existing code, to just point to a new function. The two functions will have very little overlap except checking for an error.
silverwind commentedon Feb 6, 2015
True, I'll add an second function then, definitely safer.
silverwind commentedon Feb 6, 2015
Any opinions if the address field in the object should be called
addressorip? I'm leaning more towardsaddressfor correctness, but @cjihrig suggestedip. Also, the docs usually call itaddress.cjihrig commentedon Feb 6, 2015
FWIW, I don't have a strong opinion. I just thought
addresses.addressseemed a little odd.bnoordhuis commentedon Feb 6, 2015
We call it
addressin most other places, e.g.socket.address().address.silverwind commentedon Feb 6, 2015
I'll put this up shortly for a PR, just have to test on OS X, as apparently my Windows machine isn't able to
dns.lookupfor an IPv6 (gettingENOENT), butdns.resolve6strangely works. This is onv1.xtoo. Might be a separate issue.cjihrig commentedon Feb 6, 2015
Thanks! Landed in eb30009de65a9432b59ae77a425b623df28dbd27 with a tweaked commit message.
EDIT: Someone performed a force push after I landed this. So the mentioned commit was overwritten. The actual commit is now 633a990
dns: allow dns.lookup() to return all addresses