docs: writable streams from async iterators example issue

[ronag]

Continuation of #31222

Further issues with the example. See comment:

async function pump(iterable, writable) {
  for await (const chunk of iterable) {
    // Handle backpressure on write().
    if (!writable.write(chunk)) {
      if (writable.destroyed) return;
      await once(writable, 'drain'); // BUG? This will never complete if writable is destroyed with `.destroy()`.
    }
  }
  writable.end();
}

The problem here is that it assumes that either 'drain' or 'error' will be emitted, however this is not always the case.

[ronag]

ping @mcollina

[ronag]

These are the solutions I can think of at the moment:

1. Have destroy() emit a 'drain' before 'close' if needDrain === true.

await Promise.all([
  await once(writable, 'drain')
  await once(writable, 'close')
]);

3. Have Writable emit premature close error if .destroy() is called before 'finish'

[mcollina]

See openjs-foundation/summit#216 (comment) and the rest of the thread.

I think we should add a promisified write.

cc @benjamingr @apapirovski @davidmarkclements

[ronag]

I think we should add a promisified write.

Maybe remove this await once(writable, 'drain') example then? Since it is possibly broken.

[ronag]

See openjs-foundation/summit#216 (comment) and the rest of the thread.

This example has the same problem though... 'drain' might not be emitted.

EDIT: I think most of the suggestions in that thread have the same problem in assuming that either 'error' or 'drain' will be emitted if needDrain.

[mcollina]

We might have to listen for 'finish' as well. Would you mind to send a PR to add that? I think it would fix the issue.

[ronag]

Do we and/or should we emit 'finish' when abruptly closing a writable with .destroy()? i.e. should end() and destroy() mean the same thing for a Writable?

I guess we should in order to not break... since we don't error it in this case. Just seems a little weird.

I think I would have preferred the "Have Writable emit premature close error if .destroy() is called before 'finish'" option. Though that's probably to breaking.