extension FutureIterable<T>.wait throws synchronously if iteration throws.

[stephane-archer]

extension FutureIterable<T>.wait does not work properly

from the documentation:

/// Similar to [Future.wait], but reports errors using a
/// [ParallelWaitError], which allows the caller to
/// handle errors and dispose successful results if necessary.

The following code:

Future<int> main() async {
  await process([1, 2, 3, 4, 5]);
  return 0;
}

Future<void> process(Iterable<int> messages) async {
  print("len: ${messages.length}");
  Iterable<Future<int>> futures = messages.map((message) {
    return fibo(message);
  });
  assert(messages.length == futures.length);
  try {
    print("before big wait");
    await futures.wait;
    print("after big wait");
  } catch (e) {
    print(e.runtimeType);
    print("$e from the end");
  }
  print("stop!");
}

Future<int> fibo(int message) {
  throw "Test error handling";
}

output:

flutter: len: 5
flutter: before big wait
flutter: String
flutter: Test error handling from the end
flutter: stop!

I don't see any sign of ParallelWaitError, as the documentation mentions. And the future returns an error at the first exception.

[dart-github-bot]

Summary: The FutureIterable.wait extension doesn't throw ParallelWaitError as documented. Instead, it rethrows the first encountered error, preventing expected parallel error handling.

[julemand101]

Also posted as https://stackoverflow.com/questions/79242392/extension-futureiterablet-wait-does-not-work-properly-in-dart-flutter which got an answer.

Do you still intend to have this issue open? As I mention in a comment on the SO post, you should mark foo "async" if you want it to return async errors when throwing exception.

[lrhn]

The wait getter could choose to wrap the iteration in a try/catch, but realistically it should only wrap the current access. Throwing in moveNext is not recoverable.
And that would prevent using for/in.
Don't throw synchronously during iteration, that's like the base level requirement of an iterable.

I'd also worry about the assert. It iterates the Iterable of futures, which can potentially create all the futures and not listen to them. It's probably just a lucky optimization that the default implantation of map delays the call to the map callback until you call current, which length doesn't.

[stephane-archer]

@lrhn throwing synchronously changes the behavior, I'm not sure it's the intended design. While the solution is simple to fix here, I wonder if it's the default behavior you want...

Throwing in moveNext happens because of an error in elements[1] but doesn't mean you have the same error when accessing elements[2]

/// Similar to [Future.wait], but reports errors using a
/// [ParallelWaitError], which allows the caller to
/// handle errors and dispose successful results if necessary.

here the documentation doesn't match the current behavior

@julemand101 while I found a solution, it doesn't mean the actual behavior makes sense.

[lrhn]

If you throw in moveNext, there is no next element. Every iteration in the platform libraries will exit the iteration loop in that case and consider the iterator broken. It doesn't say anywhere that moveNext "must not throw", because that's the default got every function. If they throw an Error, something is broken and all bets are off. If they throw an Exception, it must be documented, and moveNext doesn't say that it throws any exceptions.

It's very much the intended behavior to exit and fail if moveNext throws. The only question is how much cleanup should happen.

Can argue how to behave if current throws instead of moveNext, but an error has happened at a position where vi error is expected or intended. The safest thing to do is to take down the program.
But if the code uses for/in, like you're encouraged to for iteration, there is no easy to distinguish a throw in moveNext and current.

The one thing that a failure can do, is to clean up in some way, and make sure to return an asynchronous error, even if it won't be a ParallelWaitError. That should be possible. It's not clear what to do with the already found futures. Maybe just making them ignorable is enough.

The operation expects an iterable of futures, and the caller had failed to provide one. That's an error, and not an error from any of the provided futures, so it should be reported separately.

[stephane-archer]

It's very much the intended behavior to exit and fail if moveNext throws.

Please realize that my code doesn't reference any moveNext directly. While moveNext should not fail, I didn't intend this to happen, this is an implementation detail, I just wanted to see the behavior if an exception happens when calling FutureIterable.wait.
From a user perspective, FutureIterable.wait has different behavior for sync exceptions and async exceptions, witch was surprising.

[lrhn]

In this case, it isn't moveNext that fails, it's current. That's just a detail of how map is implemented, and doesn't actually matte. In either case it that for (var future in futures) { ... } will throw in the loop header.

That's not any of the futures completing with an error, it's an error happening while trying to set up the initial state.
(In this case it even happens before the first future is provided, which at least doesn't leave any futures hanging.)

It does seem that wait throws synchronously, and that might be worth changing, but doing that will not change the behavior of your example code here. That code does await futures.wait, so it would have waited for that asynchronous error and then behaved exactly the same.

If iteration of the provided iterable fails with an error, wait cannot do its job. It doesn't know which futures to work with.
It has to give up and let that error exit as the result of the call to wait.

I'll re-open as a reminder to change it to an asynchronrous error.

[stephane-archer]

I don't understand the behavior from a user perspective (Don't look at the Iterable.wait implementation)

Because assert(messages.length == futures.length); and futures is Iterable<Future<int>>

Iterable.wait work on 5 future here, not 1, then return one error if the function is sync and 5 if the function is async
The function has the same signature in both cases.

I hope you get my point, now it's up to you to decide

[lrhn]

The signature of the two functions might be the same, but one returns a Future and the other does not, rather it synchronously throws an error. That is not recommended behavior for a function returning a Future, which is why wait also shouldn't do it.

The input to wait is not asynchronous functions, it's Future objects provided by an iterable. If that Iterable doesn't deliver futures, wait can't do its thing.
If the Iterable throws, it doesn't deliver values.
The wait call doesn't work on 5 futures, it sees zero futures here, before something throws.

[stephane-archer]

The wait call doesn't work on 5 futures, it sees zero futures here, before something throws.

Iterable<Future<int>> futures is an array of 5 Future otherwise why would the length be 5? How can it be another type or another length? The assert shows this clearly.

Could you clarify how an array of Future of len 5 can be something else? If there is no future then why the length is 5?

I understand fibo throws an error synchronously but the rest of the code has such a weird response to it.

That is not recommended behavior for a function returning a Future, which is why wait also shouldn't do it.

Do we have a linter for that, do you see any legitimate use of synchronously throwing an error when returning a Future? It seems to create some strange behavior.
If even the standard library makes the mistake, I would not expect your users to be smarter