Skip to content

fix(pool): keep caller async context for queued requests - #5982

Open
mcollina wants to merge 1 commit into
mainfrom
fix/pool-queue-async-context
Open

mcollina wants to merge 1 commit into
mainfrom
fix/pool-queue-async-context

Conversation

@mcollina

@mcollina mcollina commented Oct 8, 2026

Copy link
Copy Markdown
Member

Fixes #5981

Problem

Requests that wait in the Pool queue are dispatched later from whichever callback drains it (another request's connect or drain event). Client#dispatch, and the undici:request:create diagnostics channel it publishes, therefore ran in that callback's AsyncLocalStorage context instead of the caller's.

This was already the case for saturated pools (e.g. connections: 1), but since #5799 every concurrent HTTPS request is held in the Pool queue until the first connection completes ALPN negotiation, so it now affects fetch() against any https: origin by default (h1 and h2 servers alike). MSW relies on this context and broke on Node.js v26.

Fix

Queued requests are stored as a small AsyncResource subclass that captures the caller's context, and are dispatched inside it when the queue drains. Requests dispatched directly to a free client are unaffected. BalancedPool shares PoolBase, so it is covered too.

Tests

test/issue-5981.js covers HTTPS h1, HTTPS h2 (queued during ALPN) and HTTP Pool with connections: 1. All three fail without the fix and pass with it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PU6gYCHEgVgF9WeVje8SXF

Requests waiting in the Pool queue were dispatched from whichever
callback drained it (another request's connect or drain), so
Client#dispatch and the undici:request:create diagnostics channel ran
in the wrong AsyncLocalStorage context. Since #5799 every concurrent
HTTPS request is queued during ALPN negotiation, which made this hit
fetch() by default.

Capture the caller's async context when queueing and dispatch inside it.

Fixes: #5981

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PU6gYCHEgVgF9WeVje8SXF
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@codecov-commenter

codecov-commenter commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.54%. Comparing base (e960331) to head (968ce6f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5982      +/-   ##
==========================================
+ Coverage   91.53%   91.54%   +0.01%     
==========================================
  Files         113      113              
  Lines       43597    43614      +17     
==========================================
+ Hits        39908    39928      +20     
+ Misses       3689     3686       -3     

☔ 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.

@kettanaito kettanaito left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW, looks good! Thank you so much for covering it promptly.

This branch has not been deployed

No deployments
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.

Pool loses caller's AsyncLocalStorage context for queued HTTPS requests (Undici 8)

3 participants