Stream async iteration breaks with derived streams

[gordonmleigh]

• Version: v12.4.0
• Platform: Darwin hostname.local 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64
• Subsystem: Stream

The async iterator functionality relies on a private API on destroy that accepts a second argument which is a callback. See here:

  return() {
    // destroy(err, cb) is a private API.
    // We can guarantee we have that here, because we control the
    // Readable class this is attached to.
    return new Promise((resolve, reject) => {
      this[kStream].destroy(null, (err) => {
        if (err) {
          reject(err);
          return;
        }
        resolve(createIterResult(undefined, true));
      });
    });
  }

This private API is not present on objects derived from Readable that have _destroy overridden. This means that premature exit from a for await loop which triggers the above return method will cause the program to end after running out of async continuations, as the callback is never called.

[gordonmleigh]

My bad, the documentation appears to indicate that the _destroy method should support a callback. Why is the callback API private anyway?

[addaleax]

I’m confused. destroy() is certainly not a private API – it’s documented as public API, and so the comment in our source code seems like a lie.

_destroy() is private in the sense that it should be overridden by subclasses, and only called from the streams code.

[lpinca]

destroy() callback is indeed "private"

node/lib/internal/streams/destroy.js

Line 3 in 890223d

// Undocumented cb() API, needed for core, not for public API

_destroy() callback is not.

[jasnell]

@nodejs/stream ... is there anything actionable here?

[ronag]

Was fixed in #29176