Skip to content

fix: set report.excludeNetwork=true before getReport() to avoid blocking PTR lookups#6404

Merged
lukastaegert merged 3 commits into
rollup:masterfrom
jdz321:master
Jun 4, 2026
Merged

fix: set report.excludeNetwork=true before getReport() to avoid blocking PTR lookups#6404
lukastaegert merged 3 commits into
rollup:masterfrom
jdz321:master

Conversation

@jdz321
Copy link
Copy Markdown
Contributor

@jdz321 jdz321 commented Jun 3, 2026

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

native.js calls report.getReport() during module initialization to detect the platform (isMingw32/isMusl). report.getReport() enumerates all libuv TCP handles and performs a reverse DNS (PTR) lookup on each one. When active TCP sockets are present and the remote IP's PTR query returns SERVFAIL, the call blocks until DNS times out.

This affects projects where a plugin calls fetch() during a build, leaving a keep-alive socket in undici's pool, and a later plugin import triggers native.js to load.

Fix: set report.excludeNetwork = true before calling report.getReport() and restore the previous value afterwards. For the Windows spawnSync path, excludeNetwork is set inside the child script.

Ref: nodejs/node#55576
Ref: nodejs/node#55602

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 3, 2026

@jdz321 is attempting to deploy a commit to the rollup-js Team on Vercel.

A member of the Team first needs to authorize it.

lukastaegert
lukastaegert approved these changes Jun 3, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware, nice improvement!

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 3, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
rollup Ready Ready Preview, Comment Jun 4, 2026 4:41am

Request Review

@lukastaegert lukastaegert enabled auto-merge June 3, 2026 08:28
@jdz321
Copy link
Copy Markdown
Contributor Author

jdz321 commented Jun 3, 2026

Here's a fun one, let me share how we stumbled into this bug.

While building a project with Vite 8 + vite-plugin-pwa, the Vite build itself completed quickly, but the PWA build step took an extra ~30 seconds to finish. After investigation, the root cause turned out to be a fetch() call in another Vite plugin that requests an internal IP address. Removing that plugin or changing the URL to a public address made the delay disappear.

The reason this only appears with Vite 8 is subtle: Vite 7 and earlier use rollup as the build engine, so native.js is loaded (and report.getReport() is executed) before any plugin fetch() calls happen. With Vite 8, the build engine switched to rolldown, but vite-plugin-pwa still depends on rollup internally. This means native.js is now loaded lazily — inside a closeBundle hook — which happens after the fetch() call has already left a keep-alive socket in undici's connection pool. At that point report.getReport() enumerates the live socket handles and performs a reverse DNS (PTR) lookup on the remote IP. Since the internal IP's PTR query returns SERVFAIL instead of NXDOMAIN, the DNS resolver retries until it times out, blocking the process for ~30 seconds.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.78%. Comparing base (f2a0449) to head (9daa340).

Files with missing lines Patch % Lines
native.js 83.33% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #6404   +/-   ##
=======================================
  Coverage   98.78%   98.78%           
=======================================
  Files         274      274           
  Lines       10785    10789    +4     
  Branches     2882     2882           
=======================================
+ Hits        10654    10658    +4     
  Misses         89       89           
  Partials       42       42

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

auto-merge was automatically disabled June 3, 2026 08:53

Head branch was pushed to by a user without write access

@jdz321
Copy link
Copy Markdown
Contributor Author

jdz321 commented Jun 3, 2026

the test now skips on Windows since getReport() runs inside a spawnSync child process there, making it impossible to intercept via process.report.getReport mock in the current process. The Windows fix (setting excludeNetwork in the child script) is verified by the script string itself.

@lukastaegert lukastaegert enabled auto-merge June 4, 2026 04:40
@lukastaegert lukastaegert disabled auto-merge June 4, 2026 04:59
@lukastaegert lukastaegert merged commit 91b6dc4 into rollup:master Jun 4, 2026
45 of 46 checks passed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

This PR has been released as part of [email protected]. You can test it via npm install rollup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukastaegert lukastaegert lukastaegert approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jdz321 @lukastaegert