Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readable[Symbol.asyncIterator]().next() on stream causes immediate exit (no error) #34219

Open
kwkelly opened this issue Jul 6, 2020 · 10 comments
Labels
readline Issues and PRs related to the built-in readline module. stream Issues and PRs related to the stream subsystem.

Comments

@kwkelly
Copy link

kwkelly commented Jul 6, 2020

  • v14.5.0:
  • Darwin helmholtz 19.5.0 Darwin Kernel Version 19.5.0: Tue May 26 20:41:44 PDT 2020; root:xnu-6153.121.2~2/RELEASE_X86_64 x86_64:
  • Subsystem:

What steps will reproduce the bug?

When next() is called in the program below, nothing else gets run. list.txt and list2.txt are just two line files line1\nline2 and line3\nline4

This repl.it shows the issue.
https://repl.it/repls/FrightenedPastLogin#index.js

const fs = require("fs");
const readline = require("readline");

async function main() {
  const linereader = readline.createInterface({
    input: fs.createReadStream("./list.txt"),
  });

  for await (const s of fs.createReadStream("./list2.txt")){}
  console.log(await linereader[Symbol.asyncIterator]().next())

  // nothing below here gets run
  console.log("=============");
  for await (let s of linereader) {
    console.log(s);
  }
  console.log("=============");

  return "test";
}

main()
  .then((t) => console.log(t))
  .catch((e) => console.log(e));

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

Always

What is the expected behavior?

The rest of the program should execute. The following should print

node test.js
{ value: 'line1', done: false }
=============
line2
=============
test

What do you see instead?

Nothing:

node test.js

Possibly related to #33792 #34035?

@devsnek
Copy link
Member

devsnek commented Jul 6, 2020

I believe this is a duplicate of #33463

@kwkelly
Copy link
Author

kwkelly commented Jul 6, 2020

@devsnek it may be a duplicate and probably related, but the behavior is slightly different since "node simply exits if it has nothing else to do" as in #33792

@devsnek
Copy link
Member

devsnek commented Jul 6, 2020

@kwkelly that's just because your code is waiting for a promise that will never be resolved. if you want, try replacing that line with await new Promise(() => {}).

@kwkelly
Copy link
Author

kwkelly commented Jul 6, 2020

Ok, I think I understand now. So next() is returning a promise that never fulfills. But it should always return a promise that resolves to an IteratorResult object (which may indicate done).

@devsnek
Copy link
Member

devsnek commented Jul 6, 2020

@kwkelly but since it wraps an event emitter it is technically never done, it's waiting for the next 'line' event that will never come. @nodejs/readline maybe we should remove readline's current async iterator and add a stream version of readline instead.

@ronag
Copy link
Member

ronag commented Jul 7, 2020

add a stream version of readline instead.

A stream can also be consumed as an async iterator 😄

@devsnek
Copy link
Member

devsnek commented Jul 7, 2020

@ronag yeah that's the idea. streams will exhibit backpressure so this dropping events thing won't be a problem.

@jfriend00
Copy link

The overall problem here is that readline.createInterface() immediately adds a data event listener to the stream and resumes the stream. So, if you don't synchronously add a line event listener or synchronously create the async iterator, then the readline interface will happily get data events and fire off line events, but nobody will be listening to them and they will be lost. This means that readline.createInterface() is not really compatible with other asynchronous code in many ways. The overall solution here is for the readline interface to NOT install the data listener and not resume the stream until someone installs a line listener. That would prevent loss of data when events are occurring, but nobody is listening to them.

Some other discussion of this issue here: https://stackoverflow.com/questions/62885667/why-does-this-readline-async-iterator-not-work-properly#comment111206701_62885667

@devsnek
Copy link
Member

devsnek commented Jul 14, 2020

we should just expose a stream version of it.

@jfriend00
Copy link

@devsnek - Yes, readline is a mess for this type of use (numerous bugs). A nice clean stream version would be great.

@targos targos added readline Issues and PRs related to the built-in readline module. stream Issues and PRs related to the stream subsystem. labels Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
readline Issues and PRs related to the built-in readline module. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants
@targos @ronag @devsnek @jfriend00 @kwkelly and others