Cache.add() and Cache.addAll() never settle for a response with a body

[marko1olo]

Bug Description

caches.open(...) then cache.add(request) or cache.addAll([request]) never settles when the response has a body. The promise stays pending forever — no resolution, no rejection, no timeout. A response with no body (204) settles fine, which is the discriminator.

Reproducible By

const { createServer } = require('node:http')
const { caches } = require('undici')

const server = createServer((req, res) => {
  if (req.url === '/empty') { res.writeHead(204); return res.end() }
  res.writeHead(200, { 'content-type': 'text/plain' })
  res.end('hello')
})

server.listen(0, '127.0.0.1', async () => {
  const base = `http://127.0.0.1:${server.address().port}`
  const cache = await caches.open('demo')

  const withTimeout = (p, label) => Promise.race([
    p.then(() => `${label}: settled`),
    new Promise(r => setTimeout(() => r(`${label}: STILL PENDING after 5000ms`), 5000))
  ])

  console.log(await withTimeout(cache.add(`${base}/body`), 'cache.add(200 with body)'))
  console.log(await withTimeout(cache.addAll([`${base}/body`]), 'cache.addAll([200 with body])'))
  console.log(await withTimeout(cache.add(`${base}/empty`), 'cache.add(204 no body)'))
  console.log('cache.keys() ->', (await cache.keys()).map(r => r.url))

  server.close()
})

Expected Behaviour

All three settle, and cache.keys() lists both URLs.

Actual Behaviour

cache.add(200 with body): STILL PENDING after 5000ms
cache.addAll([200 with body]): STILL PENDING after 5000ms
cache.add(204 no body): settled

cache.keys() -> [ 'http://127.0.0.1:56729/empty' ]

Only the bodyless response is ever stored. A plain fetch() of the same URL works, and cache.put(request, response) works — it is specific to add/addAll.

Where it comes from

Cache.addAll() (and Cache.add(), which delegates to it) hands fetching() a processResponseEndOfBody callback and awaits the promise it resolves — lib/web/cache/cache.js:188. It never reads the response body.

In fetchFinale, that callback only runs on one of two paths — lib/web/fetch/index.js:1129-1145:

if (internalResponse.body == null) {
  processResponseEndOfBody()
} else {
  // mcollina: all the following steps of the specs are skipped.
  // The internal transform stream is not needed.
  // See https://github.com/nodejs/undici/pull/3093#issuecomment-2050198541
  finished(internalResponse.body.stream, () => {
    processResponseEndOfBody()
  })
}

So with a body the callback waits for the body stream to finish, and nothing drains it, so it never does — addAll waits on a stream that is waiting on addAll. With body == null the callback fires immediately, which is exactly why the 204 works.

Per the spec, step 3 of that skipped section sets processResponseEndOfBody as the transform stream's flushAlgorithm, so it would fire as the body passed through rather than when a consumer finished it. The shortcut from #3093 is sound for the paths that read the body; add/addAll are the case that does not, because for them storing the response is the consumption.

I did not want to guess the right shape of the fix — draining the body inside addAll before awaiting, or restoring something equivalent to the flush hook for this path, are both plausible and the second has wider consequences.

Environment

Node v24.13.0, Windows, current main (10898087), and the same on 8.9.0. caches is exported from index.js:179.

[metcoder95]

cc: @KhafraDev @tsctx

[tomatotomata]

I was thinking about taking a first pass at this. The narrowest option seems to be draining the response body in Cache.addAll() before it waits on the end-of-body callback, then adding an issue regression test for a body-bearing response. Do you think that is the right boundary, or would you prefer restoring the flush hook in fetch for this path?