"await" doesn't wait for nested future in Future<Future<void>>

[DanTup]

My understanding is that if I await a function that returns a Future, if that Future completes with a Future, that would be awaited to. And that seems to happen for most examples. However, when working on some code in the analysis server today I found that wasn't happening. I think it might be related to FutureOr but any further attempt to simplify my example seem to "fix" the problem.

The smallest repro I have is this:

import 'dart:async';

import 'package:test/test.dart';

Future<void> main() async {
  var runner = WorkRunner(() async {
    await pumpEventQueue(times: 5000);
    print('DOING WORK');
  });
  await runner.run();
  print('FINISHED');
}

class WorkRunner {
  final Future<void> Function() work;

  WorkRunner(this.work);

  Future<void> run() => run2(work);

  FutureOr<T> run2<T>(
    FutureOr<T> Function() operation,
  ) async {
    return await operation();
  }
}

As far as I can tell, all futures are awaited. If I print(runner.run()) I see Future<Future<void>>. The output when run is:

FINISHED
DOING WORK

Removing async from run2 fixes the issue (as do all sorts of other small modifications). I suspect it's working as intended, but I don't understand why (and since there are no warnings, it feels easy to get wrong - I just happened to catch this with a test).

[DanTup]

Also slightly odd to me.. Here's a variation of the code above that has the same behaviour. But if you change the return type of run to FutureOr, the issue also goes away.

import 'dart:async';

import 'package:test/test.dart';

Future<void> main() async {
  await run();
  print('FINISHED');
}

// Changing the return type here to be FutureOr instead of Future also fixes the issue.
Future<void> run() => runImpl(doWork);

FutureOr<T> runImpl<T>(
  FutureOr<T> Function() operation,
) async {
  return await operation();
}

Future<void> doWork() async {
  await pumpEventQueue(times: 5000);
  print('DOING WORK');
}

[lrhn]

(I'll admit I'm not sure precisely what behavior is intended/desired for the examples here, and which actual happenings are good/bad. So, generally...)

My understanding is that if I await a function that returns a Future, if that Future completes with a Future, that would be awaited to.

That is not the case, and hasn't been since Dart 1. It's just not compatible (or at least manageable) with the stricter type-safety.

In fact, it's possible to have an await with a value that is just a single future, where that future is not awaited. Because awaiting it could be unsound, and being unsound is not no longer something we can accept.

Example:

void main() async {
  Object o = Future<Object?>.value(42);
  var o2 = await o;
  print(o.runtimeType); // Future<Object?>.
}

This future is not awaited because it could be unsound to do so.

The static type of await o is Object, because awaiting any non-Future type is expected to not await anything.
That means that when it sees a Future<Object?> at runtime, it sees that it's not a Future<Object>, so it cannot guarantee that the result of awaiting is an Object. (And if I change 42 to null, it won't be!)

So it treats o as a non-future value, because it's not a Future<Object>. It's the only way to be sound.

And we stopped recursively awaiting nested futures in Dart 2.0, because the logic needed to find out what the type of await o became too complex, and - honestly - inconsistent when combined with generics.

So: await (o as Future<Future<void>>) has static type Future<void> and it won't await o at runtime unless it is a Future<Future<void>>. (Here the as ensures that.) And that also means it only awaits once. If you have something which is statically known to be two nested futures, you need two awaits. (Which is fair. Also, don't nest futures!)

Now, the issues I see here are 1) that your original code infers the T of run2 to be Future<void>.
That code is:

  final Future<void> Function() work;

  Future<void> run() => run2(work);

  FutureOr<T> run2<T>(
    FutureOr<T> Function() operation,
  ) async {
    print("run2: $T"); // Future<void>
    return await operation();
  }

That is slightly weird, and bad inference.
Solving Future<void> Function() <: FutureOr<T> Function() for T does look like it could have a more immediate solution with T being void. But solving type equations in the presence of union types, without guessing, is always great fun. That is, T being Future<void> is not wrong, just not optimal.
That's probably why changing return types to FutureOr<void> fixes the issue, then it uses void as T to solve the equation FutureOr<void> Function() <: FutureOr<T> Function().
(It doesn't have to, a T of FutureOr<void> would still be a solution, but it probably finds what you want more easily.)

That inference, and nested FutureOr<Future<void>> type is likely what causes you to have nested futures to begin with. It makes the await of await operation() expect a Future<Future<void>>. When it only sees a Future<void>, it chooses to not await it (because it could complete with a non-Future<void>). Then the return does the same, and run2 completes its returned Future<Future<void>> with the future. There is no warning when run assigns that to Future<void> because everything is a subtype of void (there have been requests for lints that complain every time a Future<X> is assigned to void, or any non-Futurey type, anywhere in a type).

If I change await runner.run(); code to

  var runner = WorkRunner(() async {
    await Future.delayed(Duration(seconds:1));
    print('DOING WORK');
  });
  var p = runner.run();
  print([p].runtimeType); // List<Future<void>>
  print(p.runtimeType);   // Future<Future<void>>
  await p;

we see that the static type of runner.run() is Future<void> ([x] is a list whose element type is the static type of x.), and the returned value is a Future<Future<void>>. That's unsurprising since Future<void> is the return type of run, and Future<Future<void>> is the return type of run2 when called like that.
Awaiting that Future<Future<void>> produces a Future<void> and doesn't wait for it. (The unawaited_futures lint should catch that!)
And it then prints FINISHED before DOING WORK.

(That's on the VM, dart2js throws a type error about some null value not being a Future<void>. Not sure if that's a bug. It probably is.)

Pragmatical solution: run2<void>(work);. Don't rely on type inference when FutureOr is involved!

And possibly issue 2) that return in an async function also awaits a returned future. That's why you can return a future directly, without awaiting it, but that also means that if you do have nested futures, and you await one before returning, the return itself may await the other. Huzzah. (And dart-lang/language#870).
That doesn't happen here, but it might be why you think we do await recursively. It's really just awaiting precisely twice.

[lrhn]

All the above to say: This is "working as intended", for some definition of "intended" which leans heavily towards "how it happens to be" when it comes to tricky type inference cases.

We do not recursively await futures of futures.

So, if anything, this can be turned into a request for better inference, but it's not a bug in the implementation of futures or await (except perhaps in dart2js).

[eernstg]

Not closed: #50601.

[DanTup]

Thanks for the info! This seems to add up (I found thar removing FutureOr from all the return types and just using Future fixed the issue, as did any return value except void).

(there have been requests for lints that complain every time a Future<X> is assigned to void, or any non-Futurey type, anywhere in a type

I couldn't find this, but I would 👍 it if it would avoid this :-)

return in an async function also awaits a returned future

Oh, maybe this is part of the reason I thought this happened then. Although I also tried some other combinations I thought would be "nested Futures" and those worked:

  print(await (Completer()..complete((Completer()..complete(1)).future)).future);
  print(await Future.value(Future.value(1)));

Is there something special about Future.value and Completer.complete that means this works, or have I misunderstood?

Pragmatical solution: run2<void>(work);. Don't rely on type inference when FutureOr is involved!

Ofc, I wasn't doing this deliberately - the real code was more complex and had more indirection.. There were lots of Ts around the place and I'd happened to have used FutureOr where perhaps I shouldn't.

I guess my next question - what can I (or others) do to accidentally avoid doing this in future? Are there existing lints I could enable that would have prevented me doing this? (for example if "Never use FutureOr in x contexts" is the fix, should there be a lint to avoid you doing this accidentally?). You mentioned unawaited_futures above, but that doesn't catch my original case (and it's already enabled in analysis_server where I hit this).

[lrhn]

The Completer<T>.complete takes a FutureOr<T> as argument, and "awaits" if it's a Future<T>. That also only removes one layer of Future and also doesn't if it would be unsound.

var c1 = Completer<Object>();
var c2 = Future<Object?>.value(null);
c1.complete(c2);
c1.future.then((v) {
  print(v.runtimeType); // Future<Object?>
});

Same for the Future.value constructor.
We really, really try to avoid creating nested futures, by allowing FutureOr as argument and awaiting a future in many places, but awaiting a future with a type that might not give us a valid value is a step we won't take.

I don't know if there are existing ways to avoid this problem.
I always recommend not returning FutureOr. At least that avoids the inference problems when inferring from the context type.
Accepting FutureOr as argument is reasonable in a lot of places, but as you can see, not without its pitfalls.
(Because of dart-lang/language#870 I'd always add the await to the return statement in async functions if I expect to await something).

Maybe we should just have a lint warning if here is any Future<Future<X>> type in the program.

[lrhn]

I'll close this as "working as intended" for now. If we want to ask for better type inference, we should make a more focused request for that.

[DanTup]

I can file an issue if that makes sense - although honestly I'm not sure exactly what I'm asking for :) Given some of the examples above, if the type inference was "improved", what would the new behaviour be?

I came up with some other code that feels like it should have an error:

import 'dart:async';

abstract class A {
  FutureOr<T> work<T>();
  Future<void> run() => work(); // Why is this valid?
}

Although I'm not sure it's the same as above, because I don't think it's possible to actually implement work here (without it throwing) and any change I try to make such that I could, does produce some error.

[eernstg]

import 'dart:async';

FutureOr<T> work<T>() {
  print(T);
  return Future<T>.error(0);
}

Future<void> run() => work();

void main() => run().ignore();

This is accepted with no diagnostic messages, runs, and prints 'Future<void>' (in the current dart-pad). Hence, the inferred type argument for the invocation work() is Future<void>.

So we're returning an expression of type FutureOr<Future<void>> in the body of run. This is OK because FutureOr<Future<void>> <: Future<void>: Future<Future<void>> <: Future<void> because Future<void> <: void, and Future<void> <: Future<void> by reflexivity.

So the only part which could be wrong is that work() is inferred as work<Future<void>>() with context type Future<void>.

If it is inferred as work<S>() then we require FutureOr<S> <: Future<void>. This means that we must have Future<S> <: Future<void> as well as S <: Future<void>, that is: S <: void and S <: Future<void>. The former is trivially true (void is a top type), so we choose S == Future<void>.

I don't see anything wrong in this.

[Edit: There was no reason to change run from an => function to a {} function, undid that and fixed some typos.]

[DanTup]

I don't see anything wrong in this.

I guess it seemed to me like we had a method that was returning a FutureOr (something that might not be a Future) but we were allowed to return it from a method that must return a Future.

This isn't allowed for non-void types, and to me it felt like these were equivalent:

typedef FutureInt = Future<int>;
FutureOr<FutureInt> _getFutureOrInt() => Future<int>.value(1);
FutureInt i = _getFutureOrInt(); // 'FutureOr<Future<int>>' can't be assigned to 'Future<int>'

But, since that's apparently not the case I guess my question is now - if this can be improved (it doesn't seem ideal to accidentally not await something you think is being awaited, however it also sounds like my understanding that an await would fully await all nested futures was wrong so this could happen in other cases too) - what's the best solution. If it's "better type inference", what should be made better/changed? (I'm happy to file an issue, but I'm not sure I understand what the request is of what impact it would have on the original code... would it make things work as I'd expected? Would it make that code produce an error?)

If it's deemed nothing should change here, and the real issue is just that my assumption that I'd never get another Future from an await was wrong, then that's a reasonable conclusion too :-)

[eernstg]

we had a method that was returning a FutureOr

True, but when this is a FutureOr<Future<void>>, it is actually guaranteed to be a future. ;-)

'FutureOr<Future<int>>' can't be assigned to 'Future<int>'

That's because it isn't a subtype (and it isn't dynamic, so it isn't assignable): FutureOr<Future<int>> <: Future<int> requires Future<Future<int>> <: Future<int> and Future<int> <: Future<int>, and the former isn't true.

it doesn't seem ideal to accidentally not await something you think is being awaited

We wouldn't have that option here, because there are no async function bodies anywhere, but the underlying problems is probably that it is confusing and error prone whenever it is "forgotten" (by an upcast) that any particular expression may or will yield a future.

I think it would make sense to adjust the lint 'discarded_futures' to detect that kind of situation more aggressively (or create a new lint doing that), because that's essentially what we have: Object o = Future<Whatever>.value(whatever); makes us forget that it is a future, and void in various complex configurations could do a similar thing (as could Object or Object? or dynamic, of course).

[eernstg]

Concerning the original example, my first idea would be that a function whose body is async should never have a return type of the form FutureOr<T> for any T unless it is forced by override constraints.

We can always use Future<T> because a function whose body is async will always return a future. Hence, if we know that it will return a FutureOr<T> for any T then we probably also know that it will return a Future<T>. There are some gnarly cases around top types, but then we still know that the function will return a Future<Object?>. So you never have to use a return type of the form FutureOr<T> on these functions.

I guess this could be a lint.

In particular, we should have Future<T> run2<T>(...) {...} rather than FutureOr.

[DanTup]

Thanks - changing to Future<T> is what I did to solve the issue. I've filed #59180 for a lint to be considered. If I've described anything badly/incorrectly, feel free to edit/add comments!

[eernstg]

Thanks! I added a comment there, to the effect that it could be useful for the lint to target a slightly larger set of situations.