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

FutureProvider disposed unexpectedly in 1.0.0-dev3 #601

Closed
limenote135 opened this issue Jul 11, 2021 · 34 comments
Closed

FutureProvider disposed unexpectedly in 1.0.0-dev3 #601

limenote135 opened this issue Jul 11, 2021 · 34 comments
Labels
bug Something isn't working question Further information is requested

Comments

@limenote135
Copy link

limenote135 commented Jul 11, 2021

Describe the bug

In the following code, future returns error with Bad state: The provider AutoDisposeFutureProvider<String>#25550(example) was disposed before a value was emitted..

v0.14.0+3 works fine.

To Reproduce

final controllerProvider =
    StateNotifierProvider<Controller, List<Future<String>>>(
        (ref) => Controller(ref.read));

final futureProvider =
    FutureProvider.autoDispose.family<String, String>((ref, str) async {
  final ret = await Future.delayed(Duration(seconds: 1), () => str);
  ref.maintainState = true;
  return ret;
});

class Controller extends StateNotifier<List<Future<String>>> {
  Controller(this._read) : super([]);

  Reader _read;

  void add(String str) {
    final f = _read(futureProvider(str).future);
    state = [f, ...state];
  }
}

class MyHomePage extends ConsumerWidget {
  const MyHomePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final items = ref.watch(controllerProvider);

    return Scaffold(
      floatingActionButton: FloatingActionButton(
        onPressed: () {
          ref.read(controllerProvider.notifier).add("example");
        },
        child: const Icon(Icons.add),
      ),
      body: ListView.builder(
        itemCount: items.length,
        itemBuilder: (context, index) {
          return FutureBuilder(
            future: items[index],
            builder: (context, snapshot) {
              if (snapshot.hasError) {
                return ListTile(
                  title: Text("${snapshot.error}"),
                );
              }
              if (!snapshot.hasData) {
                return const ListTile(
                  title: Center(child: CircularProgressIndicator()),
                );
              }
              return ListTile(
                title: Text("${snapshot.data}"),
              );
            },
          );
        },
      ),
    );
  }
}

Expected behavior

Show 'example' in ListTile.

@limenote135 limenote135 added bug Something isn't working needs triage labels Jul 11, 2021
@limenote135 limenote135 changed the title FutureProvider disposed unexpectedly unexpecte in 1.0.0-dev3 FutureProvider disposed unexpectedly in 1.0.0-dev3 Jul 11, 2021
@Fraa-124
Copy link

I'm also experiencing this, with v1.0.0-dev6

@PapyElGringo
Copy link

Same with hooks_riverpod: ^1.0.0-dev.6

final messageListBoxFamily =
    FutureProvider.family.autoDispose((ref, tribuId) async {
  final secureStorage = ref.read(secureStorageProvider);
  final hiveKey = await secureStorage.read(key: 'hiveKey') ?? ''; // The error is triggered here
  final encryptionKey = base64Url.decode(hiveKey);
  Hive.registerAdapter(MessageAdapter());
  final box = await Hive.openBox<Message>('${tribuId}_messageList',
      encryptionCipher: HiveAesCipher(encryptionKey));
  return box;
});

@ablbol
Copy link

ablbol commented Sep 21, 2021

I am running into the same problem in hooks_riverpod: ^1.0.0-dev.7

I get an exception like this:
Bad state: The provider ... was disposed before a value was emitted.

Any plans on this?

@TimWhiting
Copy link
Collaborator

TimWhiting commented Sep 21, 2021

@rrousselGit Technically a future provider could be disposed before being used. (Opening a page where it is used and then pressing back before the future completes). So is this really 'a bad state'? Not sure if that is the issue reported here or if it is something else here. @PapyElGringo can you try using ref.watch instead of ref.read on the secureStorageProvider in your sample?

@rrousselGit
Copy link
Owner

This is a known bug that I will fix in due time

This error is necessary to stop provider.future. But it shouldn't be considered as uncaught

@sarakisar97
Copy link

I have the same issue, any solution?
@rrousselGit

@TimWhiting
Copy link
Collaborator

I also ran into this in one of my projects which was a command line app where I was using a container and not wanting to listen to the future just read the future. I solved it by not making my provider autodispose for now, but depending on your use case that might not be a long-term or good solution.

@marinkobabic
Copy link

This issue now exists in the released version. ref.watch instead of ref.read worked for me.

@7mada123
Copy link

Facing the same issue on 1.0.0 stable release
@marinkobabic if you use ref.watch the provider won't get dispose after the future completed

@7mada123
Copy link

I found a work around for now until the fix is out

this would work if you only use ref.read (not ref.watch or .when method) to get a value

since it is now possible to "await" all providers that emit an AsyncValue you can use "Provider" instead of "FutureProvider"

instead of

final provider = FutureProvider<bool>((ref) async {
  try {
    await futureCall();
    return true;
  } catch (e) {
    return false;
  }
});

use

final myAsyncProvider = Provider.autoDispose<Future<bool>>((ref) async {
  try {
    await futureCall();
    return true;
  } catch (e) {
    return false;
  }
});

@tomoyuki28jp
Copy link

tomoyuki28jp commented Jan 8, 2022

Faced the this issue. I solved it by not making my provider autoDispose for now.

@golane-august
Copy link

golane-august commented Jan 13, 2022

@rrousselGit I think the reason is, that the single subscriptions aren't considered when closing the provider, although they are still waiting for the response:
By adding _subscriptions.isNotEmpty || to provider_base
I could circumvent the error for the autodisposables. This should already be covered by _subscribers.isNotEmpty || but they seem to be empty.

But then I get an error error: Bad state: Stream has already been listened to, which has no stacktrace.
I also see there are a lots of TODOs in the code. Unfortunately our project relies on autoDisposables and the tests fail, because the error occurs in all places :) Would be nice, if you can give more background, what is needed to be changed in order to avoid the error.

@rrousselGit
Copy link
Owner

By adding _subscriptions.isNotEmpty || to provider_base

This change is incorrect.
_subscriptions is the list of what this provider depends on. Whereas hasListeners is about whether the provider is listened or not

The only reason this could potentially "solve" the problem in your case is because it broke autoDispose by preventing the provider from being disposed (since it now thinks that it is being listened).


I'll look into it later, but I doubt that this error is still a thing on the latest versions. It should've been fixed when I added a future.ignore() in those cases

@esenmx
Copy link

esenmx commented Jan 13, 2022

This bug prevents usage of FutureProvider as cache repository like this:

final future = FutureProvider.autoDispose.family<Response, Request>((ref, req) {
  ref.maintainState = true;
  Future.delayed(cacheDuration, () => ref.maintainState = false);
  return service.call(req);
});

Disabling autoDispose is not a good solution obviously, so I created a StateNotifier that works on top of AsyncValue and mimics the FutureProvider with RefreshIndicator compatibility(which is not valid anymore; see riverpod 2.0.0 changes). It is much more verbose but it works. So any fix greatly appreciated @rrousselGit 😄 , thanks in advance!

golane-august added a commit to golane-august/riverpod_showcase that referenced this issue Jan 13, 2022
@golane-august
Copy link

Ok thanks for clarification and the really quick reply.
Nether the less the problem still occurs in current master (v2.0.0-dev.0). See full example.

@rrousselGit
Copy link
Owner

@golane-august Your case is expected. The provider was indeed aborted before the future emitted any value.

@rrousselGit
Copy link
Owner

I should've realized it sooner, but in the reproduction example given, the exception is indeed expected.

Your issue is that you're not listening to the provider when it's marked as autoDispose.
You probably want to use either ref.watch or ref.listen instead.

To begin with, the code given in the example is not the proper way to make such architecture. Here's the fixed code:

import 'package:flutter/material.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';

void main() {
  runApp(ProviderScope(child: MaterialApp(home: MyHomePage())));
}

final controllerProvider = StateNotifierProvider<Controller, List<String>>(
  (ref) => Controller(),
);

final futureProvider =
    FutureProvider.autoDispose.family<String, String>((ref, str) async {
  final ret = await Future.delayed(Duration(seconds: 1), () => str);
  ref.maintainState = true;
  return ret;
});

class Controller extends StateNotifier<List<String>> {
  Controller() : super([]);

  void add(String str) {
    state = [str, ...state];
  }
}

class MyHomePage extends ConsumerWidget {
  const MyHomePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final items = ref.watch(controllerProvider);

    return Scaffold(
      floatingActionButton: FloatingActionButton(
        onPressed: () {
          ref.read(controllerProvider.notifier).add("example");
        },
        child: const Icon(Icons.add),
      ),
      body: ListView.builder(
        itemCount: items.length,
        itemBuilder: (context, index) {
          final str = items[index];

          return FutureBuilder(
            future: ref.watch(futureProvider(str).future),
            builder: (context, snapshot) {
              if (snapshot.hasError) {
                return ListTile(
                  title: Text("${snapshot.error}"),
                );
              }
              if (!snapshot.hasData) {
                return const ListTile(
                  title: Center(child: CircularProgressIndicator()),
                );
              }
              return ListTile(
                title: Text("${snapshot.data}"),
              );
            },
          );
        },
      ),
    );
  }
}

@rrousselGit rrousselGit added the question Further information is requested label Feb 12, 2022
@golane-august
Copy link

golane-august commented Feb 14, 2022

@rrousselGit So ref.read isn't expected to be used at all with autodisposable FutureProvider futures ? Maybe this should throw an assertion error or something with a reason phrase, as it seems like it is possible, but it leads to unexpected behavior (?). Or do you know a use case where this constellation is needed, but will not throw this disposing error? Thx ;D

@rrousselGit
Copy link
Owner

I didn't say "never use read". But there is a reason the doc is saying "prefer watch" and "avoid read" 😜

@golane-august
Copy link

golane-august commented Feb 14, 2022

I didn't say "never use read"

Sure 😅 , but for this case, I would say it. It should be disallowed by the library to use it on autodisposable futures, if it's not supposed to. For me its important to make it as predictable as possible, especially with errors.

@rrousselGit
Copy link
Owner

Using read is generally safe. Problems start when you're using it when you shouldn't

If we were to use your logic, read wouldn't exist at all. We could remove it, but this would make some scenarios really inconvenient to implement.

@rrousselGit
Copy link
Owner

In any case this error is expected, so I'll close this issue for now. If you think this is wrong, please share another reproducible example for this issue. One where the error isn't caused by a a misuse of Riverpod

@neilswinton
Copy link

@rrousselGit here are two tests where the first passes and the second fails because the provider was unexpectedly disposed. I'm new at Riverpod, Dart, and Flutter, so apologies if I've missed something obvious. In general, I've been struggling with how to unit test StreamProviders in Dart vs widget tests.

import 'package:flutter_test/flutter_test.dart';
import 'package:riverpod/riverpod.dart';

Stream<int> countGenerator(int count) {
  return Stream<int>.periodic(const Duration(microseconds: 10), (x) => x)
      .take(count);
}

final counterStreamProvider = StreamProvider<int>((ref) async* {
  await for (final item in countGenerator(10)) {
    yield item;
  }
});

final counterDisposingStreamProvider =
    StreamProvider.autoDispose<int>((ref) async* {
  await for (final item in countGenerator(10)) {
    yield item;
  }
});

void main() {
  var container = ProviderContainer();

  setUp(() {
    container = ProviderContainer();
  });

  tearDown(() => container.dispose());

  void intListener(AsyncValue<int>? from, AsyncValue<int> to) {
    expect(to, isNotNull);
  }

  test('StreamProvider test', () async {
    final streamSubscription =
        container.listen(counterStreamProvider, intListener);

    final result = await container.read(counterStreamProvider.future);
    expect(result, isNotNull);
    streamSubscription.close();
  });

  test('Disposing StreamProvider test', () async {
    final streamSubscription =
        container.listen(counterDisposingStreamProvider, intListener);

    final result = await container.read(counterDisposingStreamProvider.future);
    expect(result, isNotNull);
    streamSubscription.close();
  });
}

@rrousselGit
Copy link
Owner

rrousselGit commented Feb 15, 2022

Same story here, you probably don't want to use read and instead use watch/listen

  test('Disposing StreamProvider test', () async {
    final sub = container.listen(counterDisposingStreamProvider.future, (prev, value) {});
    final result = await sub.read();
    expect(result, isNotNull);
  });

Doing this will keep the provider until the subscription is closed.

@neilswinton
Copy link

@rrousselGit The await is on a non-future and returns AsyncLoading. Any way to wait for a data value?

I hadn't thought to read using the subscription and not the ProviderContainer. If I've got this right, the listen creates an instance of the provider which I access using the subscription; whereas, when I do a read I get a new, ephemeral instance which is disposed of immediately. If it's useful, I'm happy to turn this into something for the docs once I get it right.

@rrousselGit
Copy link
Owner

The await is on a non-future and returns AsyncLoading. Any way to wait for a data value?

Shouldn't be. Maybe you forgot to pass the generic type of to specify ".future"

@rrousselGit
Copy link
Owner

If I've got this right, the listen creates an instance of the provider

It doesn't create a new instance. It only subscribes to the cache

The problem is with "read", which does not subscribes to the cache. So since it's an autoDispose provider and it's not listened, the cache gets destroyed before the future resolved.

@neilswinton
Copy link

Thanks so much for the explanation. I did forget to specify future. I switched the test to use streams. I'm still puzzled by why the non-autodispose test sees the right stream while the autodispose provider test fails with a closed stream, but I'm also feeling like I've used enough of your time.

import 'package:flutter_test/flutter_test.dart';
import 'package:riverpod/riverpod.dart';

const listLength = 10;

Stream<int> countGenerator(int count) {
  return Stream<int>.periodic(
    const Duration(microseconds: listLength),
    (x) => x,
  ).take(count);
}

final counterStreamProvider = StreamProvider<int>((ref) async* {
  await for (final item in countGenerator(listLength)) {
    yield item;
  }
});

final counterDisposingStreamProvider =
    StreamProvider.autoDispose<int>((ref) async* {
  await for (final item in countGenerator(listLength)) {
    yield item;
  }
});

void main() {
  var container = ProviderContainer();

  setUp(() {
    container = ProviderContainer();
  });

  tearDown(() => container.dispose());

  void listener(Object? from, Object to) {
    expect(to, isNotNull);
  }

  void emitTest(ProviderSubscription<Stream<int>> subscription) {
    expect(
      subscription.read(),
      emitsInOrder(List<int>.generate(listLength, (index) => index)),
    );
  }

  test('StreamProvider test', () async {
    final streamSubscription = container.listen(
      counterStreamProvider.stream,
      listener,
    );

    // Test passes
    emitTest(streamSubscription);

    streamSubscription.close();
  });

  test('Disposing StreamProvider test', () async {
    final streamSubscription = container.listen(
      counterDisposingStreamProvider.stream,
      listener,
    );

    // Test fails with:
    // Actual: <Instance of '_AsBroadcastStream<int>'>
    // Which: emitted x Stream closed.
    //        which didn't emit an event that <0>
    emitTest(streamSubscription);
    streamSubscription.close();
  });
}

@rrousselGit
Copy link
Owner

You've refactored the code to use expect(..., emits)

The problem is, emits expectations are performed asynchronously while expect returns synchronously.
So your subscription is closed before the emits matcher actually executes.

As such writing:

final sub = container.listen(...);
expect(sub.read(), emits(value));
sub.close();

is strictly equivalent to writing:

final sub = container.listen(...);
sub.close();
expect(sub.read(), emits(value));

which should look obviously wrong.

You probably want to use await expectLater:

final sub = container.listen(...);
await expectLater(sub.read(), emits(value));
sub.close();

@neilswinton
Copy link

I put the working example into this gist: riverpod_stream_provider_test.dart. Thank you for all the good work you do. My testing world is much better now.

@jackv24
Copy link

jackv24 commented Feb 20, 2022

I've been keeping an eye on this issue for a while in hopes of a solution to an issue I've been having, but I'm not sure how to avoid using ref.read in this situation:

@override
  void initState() {
    super.initState();

    ...

    context
        .read(userComicFamily(widget.comicId).last)
        .then((userComicDoc) async {
      final currentPageId = userComicDoc?.data()?.currentPageId;

      // Get ref to current page once for centering pages on start
      final currentPageRef = currentPageId != null
          ? context.read(sharedComicPageRefFamily(SharedComicPageInfo(
              comicId: widget.comicId, pageId: currentPageId)))
          : null;

      if (currentPageRef != null) {
        // Centre on current page
        _centerPagesOnRef(currentPageRef);
      } else {
        final firstPage =
            await context.read(endPageFamily(SharedComicPagesQueryInfo(
          comicId: widget.comicId,
          descending: false,
        )).future);

        // Start at first page if no current page
        if (firstPage != null) {
          _centerPagesOnDoc(firstPage);
        } else {
          _getPages(_ScrollDirection.none);
        }
      }
    });
  }

I just need to read a few autoDispose future providers once on page open. This works fine in 0.14.0+3

@rrousselGit
Copy link
Owner

I've been keeping an eye on this issue for a while in hopes of a solution to an issue I've been having, but I'm not sure how to avoid using ref.read in this situation:

Remove the stateful widget. Move the initState logic in a separate FutureProvider which uses ref.watch

And watch that FutureProvider

jackv24 added a commit to jackv24/ComicWrap-F that referenced this issue Feb 22, 2022
@talksik
Copy link

talksik commented Jul 9, 2023

what if we want to synchronously get a futureprovider value in a asyncnotifier custom method. Currently it requires that the futureprovider actually is used in a widget, but that seems like an odd architecture.

I wonder if we can go more towards react query architecture where dependencies resolve and fetches will happen if there is a read. That should be an abstraction as that's the point of a state/data cache library.

@rrousselGit
Copy link
Owner

@talksik I'm not sure what this is about. Providers also resolve/fetch on read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.