docs: writable streams from async iterators example issue

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.

@mcollina

ping @mcollina

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

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

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

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

@benjamingr

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

I think we should add a promisified write.

cc @benjamingr @apapirovski @davidmarkclements

I think we should add a promisified write.

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

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.

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.

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.

doc: further fix async iterator example

Navigation Menu

Repository navigation

docs: writable streams from async iterators example issue #31365

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

docs: writable streams from async iterators example issue #31365

Description

Activity

ronag commented on Jan 15, 2020

ronag commented on Jan 15, 2020

mcollina commented on Jan 15, 2020

ronag commented on Jan 15, 2020

ronag commented on Jan 15, 2020

mcollina commented on Jan 15, 2020

ronag commented on Jan 15, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions