Skip to content

Detect connectivity with the system resolver instead of c-ares - #4807

Open
samuelmbabhazi wants to merge 1 commit into
Automattic:trunkfrom
samuelmbabhazi:fix/is-online-system-resolver
Open

Detect connectivity with the system resolver instead of c-ares#4807
samuelmbabhazi wants to merge 1 commit into
Automattic:trunkfrom
samuelmbabhazi:fix/is-online-system-resolver

Conversation

@samuelmbabhazi

Copy link
Copy Markdown

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() used dns.resolve(), which bypasses the operating system and queries the name servers that c-ares discovers on its own; the bundled Node reports those servers as 127.0.0.1 on 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:

  • IPv4 and IPv6 are looked up separately and the first answer wins. A single unspecified-family lookup waits for the AAAA record; on networks that drop AAAA queries that wait alone was measured above 4 seconds and would have tripped the 5 second timeout, producing the very false "offline" this change removes.
  • The timeout is cleared as soon as an answer arrives, so a quick check no longer keeps the CLI process alive for the remaining seconds.

Testing Instructions

  1. npm test -- --project common network-utils
    Covers 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.
  2. Windows reproduction from the issue: with the bundled Node, require('dns').getServers() returns ['127.0.0.1'] and dns.resolve('public-api.wordpress.com') fails, while dns.lookup() succeeds. Create a site with WordPress 6.8: before, "Cannot set up WordPress while offline"; after, the site is created.
  3. Offline check: disable the network and create a site with a specific version. The offline message still appears within 5 seconds.

Also passing locally: the full common suite (70 files, 754 tests) and the site create / site status CLI suites that mock isOnline (85 tests).

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
  • Tests added and passing

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: isOnline() false positive blocks site creation with specific WordPress versions (Node.js c-ares DNS regression)

1 participant