Query OS to find a DNS server before defaulting to Google 8.8.8.8 DNS #70
Conversation
| @@ -17,7 +17,7 @@ | |||
| "react/event-loop": "^1.0 || ^0.5", | |||
| "react/stream": "^1.0 || ^0.7 || ^0.6 || ^0.5 || ^0.4", | |||
| "react/socket": "^1.0 || ^0.8 || ^0.7", | |||
| "react/dns": "~0.4.0", | |||
| "react/dns": "~0.4.13", | |||
There was a problem hiding this comment.
Might as well target ^1.2 || ^0.4.13 since react/dns 1.0 has been out for a while
There was a problem hiding this comment.
I made this change and tests failed in Travis-CI and I have no idea what the error messages mean. Can someone help?
There was a problem hiding this comment.
After the failure in commit 90fe2c0 I think adopting react/dns 1.x is out of scope for this pull request.
|
Aside from the comment from @WyriHaximus, this looks good to me. 👍 |
|
Phake is no longer able to mock |
…t more complicated changes in Phergie
Ow yeah hehe, mock an executor instead and wrap an actual resolver around that. |
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/dnsuses 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.