Hidden bug on allReady

[JCKodel]

Dart is so freaking useless when come to generics, geez >.<

Once, I've created a FutureBuilder<void> to wait for get_it to be ready.

Since allReady is a Future<void>, I passed the result of allReady to FutureBuilder<boid>, as this:

late final Future<void> _allReady;

@override
void initState() {
  ...
  _allReady = GetIt.I.allReady();
  ...
}

@override
Widget build(BuildContext context) {
  return FutureBuilder<void>(
    future: _allReady,
    ...
}

Why? Because FutureBuilder should be fed completed (or to be completed) futures, so it won't execute the future again for each build.

But, as Dart crappy typing system allows, Future<void> is not really void. It can be anything (and it really doesn't even care if it is even Future, no complaints, no warnings, no exceptions thrown.

Since get_it returns a FutureGroup (not a FutureGroup<void>), which, for Dart, is valid

my Future<void> _allReady becomes a FutureGroup _allReady.

And guess what, wait FutureGroup() returns? Not what you intended. =(

The bug in this case is: this code will wait ONLY for the first async singleton registered in get_it, not all of them.

Tip: use these rules on analysis_options.yml to mark this kind of generics weaknesses as warnings (and, of course, don't ignore warnings):

analyzer:
  language:
    strict-casts: true
    strict-inference: true
    strict-raw-types: true

1) Since the intention of allReady is to wait for ALL async registrations to be ready, the correct would be to await the futures and then return void.

Lines 1909 and 1912 should be await futures.future. Yes. Dart will say that this is bad code (https://dart.dev/tools/linter-rules/unnecessary_await_in_return), but IT IS NOT. Without awaiting local scope futures you a) hit the Dart's weak generic mess and b) you no longer throw exceptions within this scope (exceptions will be thrown outside of it, by the caller. And the caller can be sync. Guess what happens then?).

[escamoteur]

Wow, really? I'm not sure if I introduced this a while back when doing some cleanup there. I really thought the allReady would be working. Thanks for figuring out the problem Am 5. Nov. 2024, 04:22 +0100 schrieb J.C.Ködel @.***>:

Dart is so freaking useless when come to generics, geez >.< Once, I've created a FutureBuilder to wait for get_it to be ready. Since allReady is a Future, I passed the result of allReady to FutureBuilder, as this: late final Future _allReady;

@override void initState() { ... _allReady = GetIt.I.allReady(); ... }

@override Widget build(BuildContext context) { return FutureBuilder( future: _allReady, ... } Why? Because FutureBuilder should be fed completed (or to be completed) futures, so it won't execute the future again for each build. But, as Dart crappy typing system allows, Future is not really void. It can be anything (and it really doesn't even care if it is even Future, no complaints, no warnings, no exceptions thrown. Since get_it returns a FutureGroup (not a FutureGroup), which, for Dart, is valid image.png (view on web) my Future _allReady becomes a FutureGroup _allReady. And guess what, wait FutureGroup() returns? Not what you intended. =( The bug in this case is: this code will wait ONLY for the first async singleton registered in get_it, not all of them. Tip use these rules on analysis_options.yml to mark this kind of generics weaknesses in warnings (and, of course, don't ignore warnings): analyzer: language: strict-casts: true strict-inference: true strict-raw-types: true

1. Since the intention of allReady is to wait for ALL async registrations to be ready, the correct would be to await the futures and then return void.

Lines 1909 and 1912 should be await futures.future. Yes. Dart will say that this is bad code (), but IT IS NOT. Without awaiting local scope futures you a) hit the Dart's weak generic mess and b) you no longer throw exceptions within this scope (exceptions will be thrown outside of it). Also, this is VERY, VERY BAD: https://dart.dev/tools/linter-rules/unnecessary_await_in_return since it infests our code with A LOT of bugs (Future) and removing scope of exceptions (a returned not awaited async method will delegate throws to whomever awaits the final async, which can be a sync method, hence, returning a non awaited method is bad). — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

[escamoteur]

hmmm, I just extended one of the test for allReady like this (also pushed to github) and it doesn't show anything like this

  test('ready automatic signalling for async Singletons', () async {
    try {
      final getIt = GetIt.instance;
      await getIt.reset();

      getIt.registerSingletonAsync<TestClass>(
        () async => TestClass(internalCompletion: false).init(),
      );
      getIt.registerSingletonAsync<TestClass2>(
        () async {
          final instance =
              TestClass2(internalCompletion: false, initMsDelay: 50);
          await instance.init();
          return instance;
        },
      );
      getIt.registerSingletonAsync<TestClass>(
        () async => TestClass2(internalCompletion: false).init(),
        instanceName: 'Second Instance',
      );

      expect(getIt.isReadySync<TestClass>(), false);
      expect(getIt.isReadySync<TestClass2>(), false);
      expect(
          getIt.isReadySync<TestClass>(instanceName: 'Second Instance'), false);

      await getIt.allReady();

      expect(getIt.isReadySync<TestClass>(), true);
      expect(getIt.isReadySync<TestClass2>(), true);
      expect(
          getIt.isReadySync<TestClass>(instanceName: 'Second Instance'), true);
    } on Exception catch (e) {
      print(e);
    }
  });

[JCKodel]

I'm not able to reproduce the issue in a brand-new code, and the code that had the problem was thrown away some time ago. Guess I made something wrong and did not realize that =\