Provider still triggers when not watched

[jamilsaadeh97]

Describe the bug
In a common login/logout scenario where data is fetched if there is a user and a logged out screen is shown where there's not, I found a bug where the state of a provider is AsyncError for a moment even though its not watched anymore in the widget tree.

Basically the widget tree is as follows:

1. MainScreen watches userProvider<User?> to eagerly initialize it. When complete, we show the HomeScreen.
2. HomeScreen based on the state of userProvider we either show a FavoritesScreen or a LoggedOutScreen
3. In the FavoritesScreen, we watch favoritesProvider and fetch a list of favorites. When logging out, we clear userProvider
4. When cleared, HomeScreen returns LoggedOutScreen. favoritesProvider is no longer watched.
5. This is where the error happen, favoritesProvider is "retriggered" and since there is no user, an exception is thrown

I've written a test to verify this behavior.
Moreover, if we add an observer to the container, or ref.onDispose in the favoritesProvider we can see that the provider is disposed, then updated, then disposed again.

To Reproduce

import 'dart:async';

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_test/flutter_test.dart';

final authProvider = AsyncNotifierProvider<AuthNotifier, bool>(
  AuthNotifier.new,
);

class AuthNotifier extends AsyncNotifier<bool> {
  @override
  FutureOr<bool> build() async {
    await Future.delayed(const Duration(seconds: 1));
    return true;
  }

  Future<void> logOut() async {
    await Future.delayed(const Duration(seconds: 1));
    ref.read(userProvider.notifier).logOut();
    state = const AsyncData(false);
  }
}

typedef User = ({String userID, String userName});

final userProvider = AsyncNotifierProvider<UserNotifier, User?>(
  UserNotifier.new,
);

class UserNotifier extends AsyncNotifier<User?> {
  @override
  FutureOr<User?> build() async {
    await Future.delayed(const Duration(seconds: 1));
    return (userID: "1", userName: "John Doe");
  }

  void logOut() {
    state = const AsyncData(null);
  }
}

final favoritesProvider =
    AsyncNotifierProvider.autoDispose<UserFavoritesNotifier, List<String>>(
      UserFavoritesNotifier.new,
    );

class UserFavoritesNotifier extends AutoDisposeAsyncNotifier<List<String>> {
  @override
  FutureOr<List<String>> build() async {
    //requireValue is used since this provider is eagerly initialized
    //This is where the error comes from, null check used on a null value.
    //But it should not happen since favoritesProvider is no longer watched
    final userID = ref.watch(userProvider).requireValue!.userID;

    //Use userID to fetch user's favorites
    await Future.delayed(const Duration(seconds: 1));

    return ["favorite 1", "favorite 2", "favorite 3"];
  }
}

class TestApp extends ConsumerWidget {
  const TestApp({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final asyncUser = ref.watch(userProvider);
    return switch (asyncUser) {
      AsyncData<User?>() => const MaterialApp(home: Home()),
      AsyncError() => const MaterialApp(home: Text("ERROR")),
      _ => const MaterialApp(home: CircularProgressIndicator()),
    };
  }
}

class Home extends ConsumerWidget {
  const Home({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final isGuestMode = ref.watch(userProvider).requireValue == null;
    if (isGuestMode) return const LoggedOutView();
    return const LoggedInView();
  }
}

class LoggedInView extends ConsumerWidget {
  const LoggedInView({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final asyncFavorites = ref.watch(favoritesProvider);
    return switch (asyncFavorites) {
      AsyncData(:final value) => Column(
        children: [
          ...value.map((e) => Text(e)),
          IconButton(
            onPressed: () async {
              await ref.read(authProvider.notifier).logOut();
            },
            icon: const Icon(Icons.logout),
          ),
        ],
      ),
      AsyncError() => const Text("ERROR"),
      _ => const CircularProgressIndicator(),
    };
  }
}

class LoggedOutView extends StatelessWidget {
  const LoggedOutView({super.key});

  @override
  Widget build(BuildContext context) {
    return const Center(child: Text("Logged Out"));
  }
}

void main() {
  testWidgets('Log out/Log in test', (WidgetTester tester) async {
    // Build our app and trigger a frame.
    await tester.pumpWidget(const ProviderScope(child: TestApp()));

    //Wait for the fetching of user and user favorites
    await tester.pumpAndSettle(const Duration(seconds: 2));

    final element = tester.element(find.byType(TestApp));
    ProviderScope.containerOf(element).listen(favoritesProvider, (prev, next) {
      if (next is AsyncError) {
        print("Error: ${next.error}, ${next.stackTrace}");
        throw Exception("Should not happen");
      }
    });
    // Verify that there are favorites
    expect(find.text('favorite 1'), findsOneWidget);

    // Tap the logout icon
    await tester.tap(find.byIcon(Icons.logout));

    //Wait for the log out to complete
    await tester.pumpAndSettle(const Duration(seconds: 1));

    // Verify that we're logged out.
    expect(find.text('Logged Out'), findsOneWidget);
  });
}

Expected behavior
I expect when I stop watching a provider in the widgets should dispose it immediately

[jamilsaadeh97]

I want to add that using read instead of watch in final userID = ref.watch(userProvider).requireValue!.userID; will make the test work, but this is counter intuitive and as far as I know we should never use read in the build method.

This workaround will make the log out process "work" but the log in process not work since the userID will not be refreshed.

[snapsl]

Replace

final userID = ref.watch(userProvider).requireValue!.userID;

with

final user = await ref.watch(userProvider.future);
final userID = user.userID;

[jamilsaadeh97]

Hi @snapsl

I don't think this is the solution because the null check is where the exception happens.
If I do

final user = await ref.watch(userProvider.future);
final userID = user.userID;

I will get a nullable userID and if I add a null check operator, I would get the same error.

Ultimately the problem is not the null check. It is that favoritesProvider gets re-triggered, or re-built, even if it's not watched by anything and gets re-disposed again.

If you add a provider observer, you will see that this provider, gets correctly disposed on log out since there are no watchers, gets updated, gets disposed again.

[jamilsaadeh97]

This is a much more simplified test, with no AsyncNotifiers. Just a screen where favorites are shown and a logged out screen.
I throw an Exception when showFavorites is false because I don't watch favoritesProvider when it's false.

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_test/flutter_test.dart';

final showFavoritesProvider = NotifierProvider<ShowFavoritesNotifier, bool>(
  ShowFavoritesNotifier.new,
);

class ShowFavoritesNotifier extends Notifier<bool> {
  @override
  bool build() => true;

  void dontShow() => state = false;
}

final favoritesProvider =
    NotifierProvider.autoDispose<UserFavoritesNotifier, List<String>>(
      UserFavoritesNotifier.new,
    );

class UserFavoritesNotifier extends AutoDisposeNotifier<List<String>> {
  @override
  List<String> build() {
    final showFavorites = ref.watch(showFavoritesProvider);
    if (!showFavorites) throw Exception("MUST NEVER HAPPEN");

    return ["favorite 1", "favorite 2", "favorite 3"];
  }
}

class TestApp extends ConsumerWidget {
  const TestApp({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final showFavorites = ref.watch(showFavoritesProvider);
    if (showFavorites) return const MaterialApp(home: FavoritesView());
    return const MaterialApp(home: LoggedOutView());
  }
}

class FavoritesView extends ConsumerWidget {
  const FavoritesView({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final favorites = ref.watch(favoritesProvider);
    return Column(
      children: [
        ...favorites.map((e) => Text(e)),
        IconButton(
          onPressed: () => ref.read(showFavoritesProvider.notifier).dontShow(),
          icon: const Icon(Icons.logout),
        ),
      ],
    );
  }
}

class LoggedOutView extends StatelessWidget {
  const LoggedOutView({super.key});

  @override
  Widget build(BuildContext context) {
    return const Center(child: Text("LoggedOutView"));
  }
}

void main() {
  testWidgets('Log out/Log in test', (WidgetTester tester) async {
    // Build our app and trigger a frame.
    await tester.pumpWidget(const ProviderScope(child: TestApp()));

    // Verify that we're in FavoritesView
    expect(find.text('favorite 1'), findsOneWidget);

    // Tap the logout icon
    await tester.tap(find.byIcon(Icons.logout));
    await tester.pumpAndSettle();

    // Verify that we're in LoggedOutView
    expect(find.text('LoggedOutView'), findsOneWidget);
  });
}

[rrousselGit]

That's not a bug.
Providers rebuild before widgets (because widgets depend on providers). Hence, upon popping a view, the view is disposed after the provider rebuilt.

Riverpod can't know that the listener is going to be removed soon. From Riverpod's PoV, the provider was still listened when it rebuilt it.

---

Overall the pattern you've described isn't one that's recommended. "Eager initialization" is only a widget thing. Providers shouldn't rely on that. They have the tools to await other providers to be properly initialized instead of hacking around and making assumptions.

Also, providers should be built such that they cancel their pending work upon ref.onDispose. This way, a redundant rebuild would do nothing as any pending work would be immediately aborted.

[jamilsaadeh97]

Hi @rrousselGit

Although I agree with what you said conceptually, but I think in a real-world complex app, the "should and shouldn't" lines are a bit blurred.

For instance, in the dart doc of `watch`:

//In this situation, what we do not want to do is to sort our list directly inside the build method of our UI, as sorting a list can be //expensive. But maintaining a cache manually is difficult and error prone.

//To solve this problem, we could create a separate [Provider](https://pub.dev/documentation/riverpod/latest/riverpod/Provider-//class.html) that will expose the sorted list, and use [watch]//(https://pub.dev/documentation/riverpod/latest/riverpod/Ref/watch.html) to automatically re-evaluate the list only //when needed.
//In code, this may look like:
final sortProvider = StateProvider((_) => Sort.byName);
final unsortedTodosProvider = StateProvider((_) => <Todo>[]);
final sortedTodosProvider = Provider((ref) {
  // listen to both the sort enum and the unfiltered list of todos
  final sort = ref.watch(sortProvider);
  final todos = ref.watch(unsortedTodosProvider);
  // Creates a new sorted list from the combination of the unfiltered
  // list and the filter type.
  return [...todos].sort((a, b) { ... });
});
//In this code, by using [Provider](https://pub.dev/documentation/riverpod/latest/riverpod/Provider-class.html) + [watch]//(https://pub.dev/documentation/riverpod/latest/riverpod/Ref/watch.html):
//if either sortProvider or unsortedTodosProvider changes, then sortedTodosProvider will automatically be recomputed.
//if multiple widgets depends on sortedTodosProvider the list will be sorted only once.
//if nothing is listening to sortedTodosProvider, then no sort is performed.

Especially in the last point: if nothing is listening to sortedTodosProvider, then no sort is performed.
That's not absolute because if sortProvider or unsortedTodosProvider rebuild (because a different user logged in or out for example) and a view watching sortedTodosProvider got disposed, the computation will still continue. And this will affect many things if this computation is heavy or more complex.

Riverpod can't know that the listener is going to be removed soon. From Riverpod's PoV, the provider was still listened when it rebuilt it.

Of course that makes total sense, but the problem is, the provider got disposed before rebuilding again because of an internal dependency on another provider, not because it was listened to.
I added some prints statements in the provider:

  ref
    ..onAddListener(() => print("onAddListener"))
    ..onRemoveListener(() => print("onRemoveListener"))
    ..onResume(() => print("onResume"))
    ..onCancel(() => print("onCancel"))
    ..onDispose(() => print("onDispose"));

This is the updated test code

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_test/flutter_test.dart';

final showFavoritesProvider = NotifierProvider<ShowFavoritesNotifier, bool>(
  ShowFavoritesNotifier.new,
);

class ShowFavoritesNotifier extends Notifier<bool> {
  @override
  bool build() => true;

  void dontShow() => state = false;
}

final favoritesProvider =
    NotifierProvider.autoDispose<UserFavoritesNotifier, List<String>>(
      UserFavoritesNotifier.new,
    );

class UserFavoritesNotifier extends AutoDisposeNotifier<List<String>> {
  @override
  List<String> build() {
    ref
      ..onAddListener(() => print("onAddListener"))
      ..onRemoveListener(() => print("onRemoveListener"))
      ..onResume(() => print("onResume"))
      ..onCancel(() => print("onCancel"))
      ..onDispose(() => print("onDispose"));

    final showFavorites = ref.watch(showFavoritesProvider);
    if (!showFavorites) throw Exception("MUST NEVER HAPPEN");

    return ["favorite 1", "favorite 2", "favorite 3"];
  }
}

class TestApp extends ConsumerWidget {
  const TestApp({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final showFavorites = ref.watch(showFavoritesProvider);
    if (showFavorites) {
      return const MaterialApp(home: FavoritesView());
    }
    return const MaterialApp(home: LoggedOutView());
  }
}

class FavoritesView extends ConsumerWidget {
  const FavoritesView({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final favorites = ref.watch(favoritesProvider);
    return Column(
      children: [
        ...favorites.map((e) => Text(e)),
        IconButton(
          onPressed: () => ref.read(showFavoritesProvider.notifier).dontShow(),
          icon: const Icon(Icons.logout),
        ),
      ],
    );
  }
}

class LoggedOutView extends StatelessWidget {
  const LoggedOutView({super.key});

  @override
  Widget build(BuildContext context) {
    return const Center(child: Text("LoggedOutView"));
  }
}

void main() {
  testWidgets('Log out/Log in test', (WidgetTester tester) async {
    // Build our app and trigger a frame.
    await tester.pumpWidget(const ProviderScope(child: TestApp()));

    // Verify that we're in FavoritesView
    expect(find.text('favorite 1'), findsOneWidget);

    // Tap the logout icon
    await tester.tap(find.byIcon(Icons.logout));
    await tester.pumpAndSettle();

    // Verify that we're in LoggedOutView
    expect(find.text('LoggedOutView'), findsOneWidget);
  });
}

The console will show this order:

onAddListener
onDispose
Exception
onRemoveListener
onCancel
onDispose

This is where the confusion is. Why onDispose was called if the provider was still listened to? Why was it called without onRemoveListener and onCancel being called before it?
If onDispose makes some heavy disposing, it will be called twice, and that in itself may cause an error.

Finally,

They have the tools to await other providers to be properly initialized instead of hacking around and making assumptions.

I agree with you but again, in a complex app, assumptions are made. For example, if the user got to a screen where this screen is only showed when a user is logged in, an assumption is made that userProvider has a user. If it isn't the case, the option to go to that screen won't be available from the beginning, higher in the widget tree.
To follow what you said, I have to nullify every provider's state relying on a user (or add a LoggedOutState) and if a user is null, I return null. I feel that this is also a "hack" compared to "there's no way a user has access to that screen if the state of the app is in guest mode".

I apologize for the long comment. I love riverpod and I've been using it for many years now. It's used in my production apps with thousands of users and businesses and I can't thank you enough for creating and maintaining it.

[rrousselGit]

That's not absolute because if sortProvider or unsortedTodosProvider rebuild (because a different user logged in or out for example) and a view watching sortedTodosProvider got disposed, the computation will still continue. And this will affect many things if this computation is heavy or more complex.

The provider rebuilt before the view got disposed.
It is correct for the provider to rebuild at that point in time, as it is still listened.

Heavy computation can be cancelled if need be. You can override onDispose to abort expensive work as soon as it's no-longer needed. So that's not an issue

This is where the confusion is. Why onDispose was called if the provider was still listened to? Why was it called without onRemoveListener and onCancel being called before it?
If onDispose makes some heavy disposing, it will be called twice, and that in itself may cause an error.

onDispose is called before providers rebuild. It cancels the previous work. That's normal :)
The second onDispose is to cancel the second build's work because your view got disposed. Nothing wrong either

I agree with you but again, in a complex app, assumptions are made

The fact is, you made an assumption that was incorrect due to how life-cycles work.
And Riverpod gives you the tools to not make any assumption.

Rather than throwing/returning null, you could return future; for example