Detect connectivity with the system resolver instead of c-ares - #4807
Open
samuelmbabhazi wants to merge 1 commit into
Open
Detect connectivity with the system resolver instead of c-ares#4807samuelmbabhazi wants to merge 1 commit into
samuelmbabhazi wants to merge 1 commit into
Conversation
`isOnline()` resolved public-api.wordpress.com with `dns.resolve()`, which bypasses the operating system and queries the name servers c-ares discovers on its own. On Windows, the bundled Node reports those servers as 127.0.0.1 (nodejs/node#62748), so the check failed and site creation with a specific WordPress version stopped with "Cannot set up WordPress while offline" on machines that were online. The check now uses `dns.lookup()`, the getaddrinfo path the HTTP requests that follow rely on. IPv4 and IPv6 are looked up separately and the first answer wins: a single unspecified-family lookup waits for the AAAA record, and on networks that drop AAAA queries that wait alone can exceed the five second timeout. The timeout is cleared once an answer arrives so a quick check no longer keeps the process alive for the remaining seconds. Tests cover IPv4-only, IPv6-only and fully offline answers, an IPv6 lookup that hangs after IPv4 has resolved, the timeout, that dns.resolve() is not used, and that no timer is left behind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
How AI was used in this PR
I used an AI assistant to help confirm the root cause described in the issue, measure resolver behaviour and draft the tests. I reviewed the change, ran the affected suites locally and verified the resolver timings on a real machine.
Proposed Changes
On Windows, creating a site with a specific WordPress version failed with "Cannot set up WordPress while offline" on machines that were online.
isOnline()useddns.resolve(), which bypasses the operating system and queries the name servers that c-ares discovers on its own; the bundled Node reports those servers as127.0.0.1on affected Windows installs (nodejs/node#62748), so the probe failed while the system resolver worked.The check now uses
dns.lookup(), the getaddrinfo path that the HTTP requests Studio makes right after rely on. Two details matter for reliability:Testing Instructions
npm test -- --project common network-utilsCovers IPv4-only, IPv6-only and fully offline answers, an IPv6 lookup that hangs after IPv4 resolved, the timeout, that
dns.resolve()is no longer called, and that no timer is left behind.require('dns').getServers()returns['127.0.0.1']anddns.resolve('public-api.wordpress.com')fails, whiledns.lookup()succeeds. Create a site with WordPress 6.8: before, "Cannot set up WordPress while offline"; after, the site is created.Also passing locally: the full
commonsuite (70 files, 754 tests) and thesite create/site statusCLI suites that mockisOnline(85 tests).Pre-merge Checklist