stream: async iterator detaches its 'error' listener before a pending error is emitted (autoDestroy: false), causing an unhandled 'error' #67418

Version

v22.23.1, v24.22.2, v25.9.0. The code involved is unchanged on main.

Platform

Linux 6.18 x86_64 (WSL2)

Subsystem

stream

What steps will reproduce the bug?

   import { Readable } from 'node:stream';

   const stream = new Readable({
     read() {},
     autoDestroy: false,
     destroy(err, cb) {
       // Finishes asynchronously, like fs streams closing their file.
       setTimeout(() => cb(err), 10);
     },
   });

   stream.destroy(new Error('boom'));

   try {
     for await (const chunk of stream) {}
   } catch (err) {
     console.log('iterator threw:', err.message);
   }

   await new Promise((resolve) => setTimeout(resolve, 50));
   console.log('done');

((The same thing happens with fs streams. Creating a stream with an already-aborted signal destroys it in the constructor, and autoClose: false sets autoDestroy: false))

How often does it reproduce? Is there a required condition?

Always, when all of these hold:

1. The stream is destroyed with an error before iteration starts.
2. _destroy finishes asynchronously, so the 'error' event is not yet queued when the iterator starts. fs streams always do this.
3. autoDestroy is false.

With autoDestroy: true, or with a _destroy that calls back synchronously, there is no crash.

What is the expected behavior? Why is that the expected behavior?

iterator threw: boom
done

If there's a destroyed stream, I'd expect the exception to be thrown on iteration. And not bubble up elsewhere as uncought if I have try/catch around the iteration.

The iterator has already delivered the error to the consumer, who handled it. The same error should not then be raised again as an unhandled 'error' event. With autoDestroy: true, Node behaves this way.

Alternatively, it might also be thrown from the constructor - that'd also make some sense; that way we'd not even get to iteration.

What do you see instead?

   iterator threw: boom
   node:events:497
         throw er; // Unhandled 'error' event
         ^

   Error: boom
       at file:///repro.mjs:12:16
       ...
   Emitted 'error' event on Readable instance at:
       at emitErrorNT (node:internal/streams/destroy:170:8)

The process exits. The fs version fails the same way with AbortError / ABORT_ERR.

Additional information

AI generated (but verified) notes bellow:

The order of events:

1. destroy(err) sets errored and destroyed. _destroy is still running, so the 'error' event will only be emitted later, from emitErrorCloseNT.
2. createAsyncIterator calls eos(), which adds an 'error' listener. Because the stream is already destroyed, eos() also schedules onclosed on the next tick.
3. onclosed reads the stored error and passes it to the iterator directly, without waiting for the 'error' event. The iterator throws.
4. The iterator's finally (finalize() on main) checks (error === undefined || autoDestroy). With autoDestroy: false, it takes the detach branch: stream.off('readable', next); cleanup(). This removes the 'error' listener.
5. _destroy calls back, and emitErrorNT emits 'error' on a stream with no listeners.

When the stream is destroyed during iteration instead, the iterator only learns about the error through the 'error' event, so its listener is still attached. Only an already-destroyed stream with a pending emission hits this.

Possible fix: in the detach branch, if the stream is errored but the error hasn't been emitted yet (errored && !errorEmitted), keep a listener until 'error'/'close', or don't detach the eos listener until then.

This came up in a real crash (earendil-works/pi#11956). We worked around it by not passing the signal to the stream.