search

szmarczak / cacheable-lookup

A cacheable dns.lookup(…) that respects TTL :tada:
MIT License
193 stars 29 forks source link

Fallback to dns.lookup for IPv4 & IPv6 separately #43

Closed pimterry closed 3 years ago

pimterry commented 3 years ago

This fixes #42.

I'm not sure how to test it without adding significantly more complex logic to the dns.lookup mock, and I'm not sure if you'd want that? For now I've just updated the existing tests that cover this, and they still seem to pass happily.

I have tested this manually, using the /etc/hosts configuration and repro code from #42. With this change, the code there now returns the expected result: returning the same IPv6 result as dns.lookup('localhost', { family: 6 }.

I'm not totally confident that double-requesting like this won't produce some other strange side-effects in other edge cases. I'm not really familiar with this codebase or the oddities of the DNS API, so a detailed review would be very helpful!

szmarczak commented 3 years ago

Thanks for the PR! Looks good. I'm quite tired though, will take a fresh look tomorrow.

pimterry commented 3 years ago

Hi @szmarczak any thoughts on this?

szmarczak commented 3 years ago

Sorry for delay. Thanks for the fix! 🙌🏼