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

async* stream never ends if the generator's return is caused by an exception #47342

Open
vexcat opened this issue Sep 30, 2021 · 4 comments
Open
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dev-compiler-async P2 A bug or feature request we're likely to work on web-dev-compiler web-triage-1 priority assigned

Comments

@vexcat
Copy link

vexcat commented Sep 30, 2021

Dart SDK version: 2.14.2 (stable) (Wed Sep 15 12:32:06 2021 +0200) on "linux_x64"
OS: Linux
Platforms: desktop (working), web on chrome (bug)

Code:

int idx = 0;
Future<int> getValueExceptionOnEnd() async {
  if (idx < 4) {
    return idx++;
  } else {
    throw RangeError("no more ints for you!");
  }
}

Stream<int> onlyWorksOnDesktop() async* {
  try {
    while (true) {
      yield await getValueExceptionOnEnd();
    }
  } on RangeError {
    print("all done! dart should close the stream after this.");
    return;
  }
}

Future<int?> _allPlatformsHelper() async {
  try {
    return await getValueExceptionOnEnd();
  } on RangeError {
    return null;
  }
}

Stream<int> worksOnAllPlatforms() async* {
  while (true) {
    final value = await _allPlatformsHelper();
    if (value == null) break;
    yield value;
  }
  print("all done! dart should close the stream after this.");
}

void bugDemo() async {
  idx = 0;
  await for (var value in worksOnAllPlatforms()) {
    print(value);
  }
  print("worksOnAllPlatforms()'s stream closed.");

  idx = 0;
  await for (var value in onlyWorksOnDesktop()) {
    print(value);
  }
  //This never runs when developing for the web with flutter
  //(though, interestingly, it does work with dart2js.)
  print("onlyWorksOnDesktop()'s stream closed.");
}

bugDemo is being run in a Flutter app targeting desktop & the web.

Desktop output (expected):

0
1
2
3
all done! dart should close the stream after this.
worksOnAllPlatforms()'s stream closed.
0
1
2
3
all done! dart should close the stream after this.
onlyWorksOnDesktop()'s stream closed.

Web output:

0
1
2
3
all done! dart should close the stream after this.
worksOnAllPlatforms()'s stream closed.
0
1
2
3
all done! dart should close the stream after this.
@lrhn
Copy link
Member

lrhn commented Oct 5, 2021

I'm not seeing the reported web output in dart2js (dartpad.dev or tip-of-tree dart2js).

Are you using dartdevc to compile for the web?

@lrhn lrhn added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Oct 5, 2021
@vexcat
Copy link
Author

vexcat commented Oct 5, 2021

Yes, this only affects dartdevc. I also cannot reproduce the issue when using dart2js.

@nshahan
Copy link
Contributor

nshahan commented Oct 6, 2021

@vexcat Thanks for the report and the reproduction! This looks like a duplicate of #35624.

We have been tracking issues related to async timings in DDC compiled applications for some time but fixes have always been at a lower priority. To help us prioritize can I ask how you ran into this bug? Did you discover it through the use of some public package? Are you working with a flutter web project or using another other dart web framework?

@nshahan nshahan added the web-triage-0 repro is available label Oct 6, 2021
@vexcat
Copy link
Author

vexcat commented Oct 6, 2021

I was working on a flutter web project for analyzing a binary file. I encountered the issue while writing my own code using streams; this isn't an issue with a library function. I had written a class, AsyncByteStream, which made a Stream<ByteData> accessible via a function of signature Future<ByteData> load(int length), making parsing the byte stream a bit more convenient by letting the caller decide what size chunks it needs. I decided to handle EOF of the wrapped Stream<ByteData> in this class by throwing an exception from load(). I wanted to go from my Stream<ByteData> input -> AsyncByteStream for parsing -> Stream<ParsedData> generated using an async* function as output.

It looked something like this:

Stream<ParsedData> parse(AsyncByteStream stream) async* {
  try {
    while(true) {
      final header = await stream.load(16);
      final length = header.getUint32(0, Endian.little);
      //parsing happens in this constructor
      yield ParsedData(await stream.load(length));
    }
  } on RangeError {
    //EOF
    return;
  }
}

I ended up abandoning the idea of an AsyncByteStream class and just had parse() accumulate data from a Stream itself and yield when enough data was present, avoiding this exception issue.

@nshahan nshahan added P2 A bug or feature request we're likely to work on web-triage-1 priority assigned and removed web-triage-0 repro is available labels Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dev-compiler-async P2 A bug or feature request we're likely to work on web-dev-compiler web-triage-1 priority assigned
Projects
None yet
Development

No branches or pull requests

4 participants