`ReadableStreamFrom`: unguarded delayed `controller.close()` throws uncatchable `ERR_INVALID_STATE` when an async-iterable body is cancelled

[aqm857886159]

Bug Description

extractBody routes any async-iterable body — including a Node.js Readable, which is async-iterable — to ReadableStreamFrom (lib/core/util.js). That function has a delayed close() with no guard, and a cancel() that records no state:

pull (controller) {
  return iterator.next().then(({ done, value }) => {
    if (done) {
      return queueMicrotask(() => {
        controller.close()               // <-- unguarded
        controller.byobRequest?.respond(0)
      })
    }
    // ...
  })
},
cancel () {
  return iterator.return()               // <-- sets no closed flag
},

The race:

1. pull() calls iterator.next(), which returns a pending promise.
2. The consumer cancels (reader.cancel()), so the ReadableStream transitions to closed.
3. cancel() calls iterator.return(), which makes the in-flight next() settle with done: true.
4. pull()'s continuation runs queueMicrotask(() => controller.close()) — against a controller that is already closed — and throws ERR_INVALID_STATE.

Because the throw happens inside a queueMicrotask callback after the read() promise has already settled, it is attached to no promise and no try/catch or .catch() at the call site can observe it. It goes straight to uncaughtException, which in many hosts (Electron's main process, for one) means a crash dialog or process exit.

This is not the response-body teardown path. That one has been guarded by readableStreamClose() since v6.0.0, which is why existing reports like #1564, #2009 and #1137 (all in fetchParams.controller.resume / cancelBody) look similar but are a different code path. This one is the request-body / new Response(body) side.

Reproduction

No createServer is needed: the bug is in body extraction, which happens before anything is dispatched. next() is resolved by hand so the ordering in steps 1–4 above is deterministic rather than timing-dependent — it fails 5/5 runs.

Standalone reproduction script:

'use strict'

// node --test test-repro.js

const { test } = require('node:test')
const assert = require('node:assert')
const { Response } = require('undici')

test('cancelling an async-iterable body must not throw ERR_INVALID_STATE', { timeout: 30000 }, async (t) => {
  const uncaught = []
  const onUncaught = (err) => uncaught.push(err)

  // node:test installs its own handler and would count the throw as a crash;
  // swap it out so we can assert on it instead.
  const previous = process.listeners('uncaughtException')
  for (const l of previous) process.off('uncaughtException', l)
  process.on('uncaughtException', onUncaught)
  t.after(() => {
    process.off('uncaughtException', onUncaught)
    for (const l of previous) process.on('uncaughtException', l)
  })

  // An async iterable whose next() we settle by hand, to control the ordering
  // of "consumer cancels" vs "in-flight next() resolves with done: true".
  let resolveNext
  const iterable = {
    [Symbol.asyncIterator]: () => ({
      next: () => new Promise((resolve) => { resolveNext = resolve }),
      return: () => Promise.resolve({ done: true, value: undefined })
    })
  }

  let caughtAtCallSite = null
  const reader = new Response(iterable).body.getReader()

  // 1. Start pull() -> iterator.next(), which now hangs on our promise.
  reader.read().catch((err) => { caughtAtCallSite = err })
  await new Promise((resolve) => setImmediate(resolve))

  // 2. Consumer cancels. cancel() is `return iterator.return()` and records no
  //    state, but the stream itself is now closed.
  await reader.cancel()

  // 3. The in-flight next() settles with done: true, so pull() schedules
  //    queueMicrotask(() => controller.close()) against a closed controller.
  resolveNext({ done: true, value: undefined })

  await new Promise((resolve) => setTimeout(resolve, 100))

  assert.deepStrictEqual(
    uncaught.map((e) => e.code), [],
    `expected no uncaughtException, got: ${uncaught.map((e) => e.stack).join('\n')}`
  )
  // The throw escapes from a microtask after the read promise already settled,
  // so no amount of try/catch or .catch() at the call site can observe it.
  assert.strictEqual(caughtAtCallSite, null)
})

The same failure occurs with the global Response (Node's bundled undici), so it does not depend on installing the standalone package.

Expected Behavior

Cancelling a stream whose body came from an async iterable should tear down quietly. pull()'s delayed close() should be a no-op once the controller is already closed — the same contract the fetch response path already gets from readableStreamClose().

Actual Behavior

An uncaughtException escapes, and the call site sees nothing:

uncaughtException count : 1
uncaught codes          : ["ERR_INVALID_STATE"]
caught at call site     : null

TypeError [ERR_INVALID_STATE]: Invalid state: ReadableStream is already closed
    at ReadableByteStreamController.close (node:internal/webstreams/readablestream:1162:13)
    at /tmp/undici-rsf-repro/node_modules/undici/lib/core/util.js:664:26
    at node:internal/process/task_queues:149:7
    at AsyncResource.runInAsyncScope (node:async_hooks:214:14)
    at AsyncResource.runMicrotask (node:internal/process/task_queues:146:8)

Logs & Screenshots

Verified by reading the source at each tag. The async/.then() style differs between 6.x and 7.x+, but the unguarded queueMicrotask(() => controller.close()) and the flagless cancel() are identical in all of them:

undici	controller.close() in ReadableStreamFrom	guarded?
6.19.8	lib/core/util.js:481	no
7.29.0	lib/core/util.js:636	no
8.10.0	lib/core/util.js:664	no
main	lib/core/util.js:663-664	no

Environment

• OS: macOS 26.5.1 (arm64)
• Node.js version: v24.13.1
• undici version: 8.10.0 (also reproduced on Node 24.13.1's bundled undici, at node:internal/deps/undici/undici:1538)

Additional context

Suggested fix. readableStreamClose() (lib/web/fetch/util.js:968) already does exactly the right thing — it wraps the same two calls and swallows both 'Controller is already closed' and 'ReadableStream is already closed'. ReadableStreamFrom hand-rolls those two lines without it.

Reusing the helper directly would be circular: lib/web/fetch/util.js:9 already requires ../../core/util for ReadableStreamFrom. So the options are to inline the same guard in lib/core/util.js, or move readableStreamClose down into core and re-export it. Inlining looks like the smaller change:

return queueMicrotask(() => {
  try {
    controller.close()
    controller.byobRequest?.respond(0)
  } catch (err) {
    // The consumer may have cancelled while iterator.next() was still
    // in flight, which closes the controller before we get here.
    if (!err.message.includes('Controller is already closed') &&
        !err.message.includes('ReadableStream is already closed')) {
      throw err
    }
  }
})

I applied that patch to the installed 8.10.0 and re-ran: the repro goes from failing 5/5 to passing 5/5, a 3 MB fs.createReadStream round-trips through new Response() byte-identical, and an async generator that ends normally still closes (await new Response(it).text() === 'abc'). Happy to open a PR with this plus a regression test if the approach looks right — including if you'd prefer the flag-in-cancel() variant instead of catching by message.

Precedent. #5105 fixed this exact shape (cancel() closes the controller, a later path closes it again → uncaught ERR_INVALID_STATE) in WebSocketStream, by routing the second close through readableStreamClose(). This is the same bug in ReadableStreamFrom.

Related but distinct. nodejs/node#64529 is the sibling race in Node's own Readable.toWeb() adapter (open; fix PRs #62773 and #64766 both still open). Same error code and same "throws from a microtask, uncatchable" shape, different adapter. Worth noting because Readable.toWeb(stream) is the natural workaround for this issue and it currently lands on that one instead.

How we hit it. Nomi, a local-first Electron video workbench, serves local video files to the renderer over a custom protocol, with the body built as new Response(fs.createReadStream(path)). Every seek in the player cancels the in-flight body — exactly step 2 above. Under I/O contention it reproduced in 79 of 2400 cancels (3.3%) on macOS; at concurrency 1 it dropped to 2/2400 (0.083%), which fits the mechanism: the race window is roughly the duration of one disk read, so it only opens up when reads are slow enough to still be in flight when the cancel arrives. Users saw it as a main-process crash dialog that we had no way to catch.

We worked around it downstream by owning the stream ourselves instead of handing an async iterable to Response, so this report is not blocking us — but the same shape is reachable by anyone passing a Node Readable to Response/fetch, and it is not obvious from the call site that cancellation can crash the process.