[mcollina]

A synchronous request validation failure (e.g. an invalid header value) can leave a free Client permanently marked as needing drain inside a Pool. Subsequent valid requests stay queued forever even though no request is running.

Client.dispatch constructs its Request before enqueuing it. When validation throws, DispatcherBase#dispatch calls handler.onResponseError and returns false. PoolBase#dispatch treats that false as backpressure and sets dispatcher[kNeedDrain] = true, but nothing was enqueued, so the client never becomes busy and never emits the drain event that would clear the flag. The client stalls permanently.

This distinguishes a synchronous validation error from real backpressure by only marking a client as needing drain when it is actually saturated. For custom dispatchers (which expose no kBusy signal) false still means backpressure, preserving existing behavior.

Fixes #6054

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.57%. Comparing base (f4e4e45) to head (53c69c7).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6081   +/-   ##
=======================================
  Coverage   91.57%   91.57%           
=======================================
  Files         113      113           
  Lines       43938    44022   +84     
=======================================
+ Hits        40235    40315   +80     
- Misses       3703     3707    +4     

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

[kilisamemarisaaa]

Independent Windows verification of 53c69c7abdb42901ae3296a9fec3c7e2944dd75c against its merge base f4e4e4507a1fe5407093cb3d58ca240882897a3e:

The 14-case public Pool.request() probe below gives base: 12 fail / 2 pass; PR: 14 pass. Besides the warm-client path in this PR's regression test, it exercises cold clients and invalid requests queued behind real in-flight responses. Each mode covers invalid header values and methods, with connections: 1 and 2; the warm two-client case rejects enough requests to exhaust both slots on the base. The invalid requests must reject with UND_ERR_INVALID_ARG, never reach the server, and leave the following valid request able to complete.

The two ordinary-backpressure controls pass on both revisions: all available connections are held by server responses, the next valid request stays queued until those responses are released, and then completes. On the PR all cases finish with zero running/pending/queued requests. The queued-validation case would be useful additional regression coverage for the changed drain loop.

Also ran test/pool.js (39 passed), test/node-test/agent.js, test/node-test/balanced-pool.js, test/pool-retired-clients.js, and test/round-robin-pool.js (84 passed together), plus npm run lint (passed). Dependencies installed with npm ci --ignore-scripts. Environment: Windows x64, Node.js 24.12.0, npm 11.6.2, source version 8.12.3.

This is scoped evidence for stock HTTP/1 pools and these adjacent tests. I have not independently verified HTTP/2, custom dispatchers without kBusy, the full project suite, or the 7.x backport, and am leaving the broader compatibility decision to the maintainers.

Complete standalone probe (Node built-ins only, plus the checked-out Undici)

Save as review.cjs outside the checkout; run node review.cjs /absolute/path/to/checkout against each revision. It uses Node's test runner through node:test; the worktree argument is a script argument, so do not add the CLI --test flag.

'use strict'
// Run: node review-probes/undici-6081-pool-matrix.cjs <worktree>
const { test } = require('node:test')
const assert = require('node:assert/strict')
const { createServer } = require('node:http')
const { once } = require('node:events')
const { setImmediate: nextTurn } = require('node:timers/promises')
const { resolve } = require('node:path')
const { Pool } = require(resolve(process.argv[2]))
const badOptions = {
  header: { method: 'GET', headers: { 'x-invalid': '\n' } },
  method: { method: 'bad method' }
}
async function bounded (promise, label) {
  let timer
  try {
    return await Promise.race([promise, new Promise((resolve, reject) => {
      timer = setTimeout(() => reject(new Error('TIMEOUT: ' + label)), 1000)
    })])
  } finally { clearTimeout(timer) }
}
for (const connections of [1, 2]) {
  for (const mode of ['cold', 'warm', 'queued', 'backpressure-control']) {
    for (const bad of mode === 'backpressure-control' ? ['none'] : Object.keys(badOptions)) {
      test(`${mode}/${bad}/connections=${connections}`, { timeout: 10000 }, async t => {
        const received = []
        const held = []
        let allHeld
        const holding = new Promise(resolve => { allHeld = resolve })
        const server = createServer((req, res) => {
          received.push(req.url)
          if (req.url === '/hold') {
            held.push(res)
            if (held.length === connections) allHeld()
          } else res.end('ok')
        })
        server.listen(0, '127.0.0.1')
        await once(server, 'listening')
        const pool = new Pool(`http://127.0.0.1:${server.address().port}`, { connections, pipelining: 1 })
        t.after(async () => {
          await pool.destroy()
          server.closeAllConnections()
          await new Promise(resolve => server.close(resolve))
        })
        const request = options => pool.request({ path: '/after', method: 'GET', ...options })
          .then(async response => {
            assert.equal(response.statusCode, 200)
            return response.body.text()
          })
        const invalid = () => request({ path: '/invalid', ...badOptions[bad] })
          .then(() => { throw new Error('invalid request succeeded') }, error => {
            assert.equal(error.code, 'UND_ERR_INVALID_ARG')
          })
        if (mode === 'warm') {
          assert.deepEqual(await Promise.all(Array.from({ length: connections }, () => request({ path: '/warm' }))), Array(connections).fill('ok'))
          await nextTurn()
          assert.equal(pool.stats.free, connections)
        }
        if (mode === 'queued' || mode === 'backpressure-control') {
          const busy = Array.from({ length: connections }, () => request({ path: '/hold' }))
          const busyDone = Promise.all(busy)
          busyDone.catch(() => {})
          await bounded(holding, 'server received held requests')
          const errors = bad === 'none' ? Promise.resolve() : Promise.all(Array.from({ length: connections }, invalid))
          errors.catch(() => {})
          const valid = request()
          valid.catch(() => {})
          assert.equal(pool.stats.queued, (bad === 'none' ? 0 : connections) + 1)
          assert.equal(received.includes('/after'), false)
          for (const response of held) response.end('ok')
          await bounded(busyDone, 'held responses completed')
          await bounded(errors, 'queued invalid requests rejected')
          assert.equal(await bounded(valid, 'valid request after queue drained'), 'ok')
        } else {
          for (let i = 0; i < connections; i++) await bounded(invalid(), 'direct invalid request rejected')
          assert.equal(await bounded(request(), 'valid request after validation error'), 'ok')
        }
        assert.equal(received.includes('/invalid'), false)
        assert.equal(received.filter(path => path === '/after').length, 1)
        await nextTurn()
        assert.equal(pool.stats.running, 0)
        assert.equal(pool.stats.pending, 0)
        assert.equal(pool.stats.queued, 0)
        console.log(JSON.stringify({ mode, bad, connections, received, free: pool.stats.free }))
      })
    }
  }
}