search

phergie / phergie-irc-client-react

IRC client library built on React
BSD 3-Clause "New" or "Revised" License
56 stars 26 forks source link

Query OS to find a DNS server before defaulting to Google 8.8.8.8 DNS #70

Closed bkidwell closed 2 years ago

bkidwell commented 5 years ago

In release 0.4.13 of reactphp/dns (Feb 2018) they added support for asking the host OS for a DNS server address instead of requiring that the calling module provide one.

We should use this instead of hard-coding Google 8.8.8.8 as the only DNS server Phergie will ever use.

This patch will cause blocking IO the first time you do a DNS query, as the reactphp/dns uses the appropriate method to get the DNS server address from the host OS. This blocking should only ever happen once per Phergie launch, and should take less than a second, I think.

Note: without a patch like this one, Phergie will fail hard when trying to find and connect to a private IRC server on the local network if the server isn't available on the public Internet.

elazar commented 5 years ago

Aside from the comment from @WyriHaximus, this looks good to me. 👍

elazar commented 5 years ago

Phake is no longer able to mock Resolver because it's been made final in the newer version of react/dns. The newer version does provide a ResolverInterface interface to typehint against, but that only exists in the newer version. It may take further consideration to come up with a solution that supports both versions of react/dns.

WyriHaximus commented 5 years ago

Phake is no longer able to mock Resolver because it's been made final in the newer version of react/dns. The newer version does provide a ResolverInterface interface to typehint against, but that only exists in the newer version. It may take further consideration to come up with a solution that supports both versions of react/dns.

Ow yeah hehe, mock an executor instead and wrap an actual resolver around that.