Skip to content

fix: prevent node-fetch test server close from hanging on Windows#5246

Merged
mcollina merged 1 commit intomainfrom
fix/5178-flaky-node-fetch-test
May 6, 2026
Merged

fix: prevent node-fetch test server close from hanging on Windows#5246
mcollina merged 1 commit intomainfrom
fix/5178-flaky-node-fetch-test

Conversation

@mcollina
Copy link
Copy Markdown
Member

@mcollina mcollina commented May 6, 2026

Description

The test/node-fetch/main.js suite was flaking on Windows with:

test at test\node-fetch\main.js:1:1
✖ D:\a\undici\undici\test\node-fetch\main.js (696.5575ms)
  'test failed'
Test results (58 passed, 1 failed)

This was caused by TestServer.stop() calling server.close() and awaiting the 'close' event, but on Windows, lingering socket connections could prevent the 'close' event from firing. The after hook would hang until the test runner's module-level timeout killed it, causing the entire test module to fail.

Fix

Call server.closeAllConnections() before server.close() in TestServer.stop() to ensure all active connections are destroyed before attempting to close the server. This is a standard pattern for test server teardown and is safe because closeAllConnections() was added in Node.js 18.2.0 and the project already requires Node.js 18+.

Testing

  • Full npm run test:node-fetch passes (176 pass, 13 skipped, 0 fail)
  • Fix addresses the root cause of the Windows CI timeout

The TestServer.stop() method called server.close() and waited for
the 'close' event, but on Windows lingering socket connections could
prevent the event from firing, causing the entire test module to time
out with 'test failed'.

Adding closeAllConnections() before close() ensures all active
connections are destroyed, allowing the server to close promptly.

Fixes #5178
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.27%. Comparing base (4f8f814) to head (b44e031).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5246   +/-   ##
=======================================
  Coverage   93.27%   93.27%           
=======================================
  Files         110      110           
  Lines       36366    36366           
=======================================
  Hits        33922    33922           
  Misses       2444     2444           

☔ View full report in Codecov by Sentry.
📢 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.

@trivikr
Copy link
Copy Markdown
Member

trivikr commented May 6, 2026

Fixes: #5178

@mcollina mcollina merged commit 5486712 into main May 6, 2026
35 checks passed
@mcollina mcollina deleted the fix/5178-flaky-node-fetch-test branch May 6, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants