riverpod icon indicating copy to clipboard operation
riverpod copied to clipboard

Provider watching itself causes a loop because of `.persist`

Open lucavenir opened this issue 6 months ago • 6 comments

Describe the bug Self-watching a provider that persists itself causes the framework to loop the provider executions. Removing or commenting the persist action immediately stops the loop, even with a hot reload.

To Reproduce

Fully reproducible repo, here.

TL;DR: make a family provider with a page parameter; let it persist itself and watch the previous page to fetch the last item id (i.e. ref.watch(provider(page-1))). The logs show the framework is stuck computing the last elements with no way out.

Details
// in this file, uncomment the following lines to reproduce the issue, even with a hot reload
// import 'dart:convert';
import 'dart:math' as math;

import 'package:flutter_riverpod/experimental/persist.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';
// import 'package:riverpod_persist_problem/storage_provider.dart';

part 'history_provider.g.dart';

@riverpod
class History extends _$History with Persistable<List<int>, String, String> {
  @override
  FutureOr<List<int>> build(int id, {required int page}) async {
    // final storage = await ref.watch(storageProvider.future);
    print("executing historyProvider($page)");

    // await persist(
    //   key: 'history-$id-$page',
    //   storage: storage,
    //   encode: (state) => jsonEncode(state),
    //   decode: (encoded) {
    //     final decoded = jsonDecode(encoded) as List<Object?>;
    //     return decoded.cast<int>();
    //   },
    // );
    // print("stored historyProvider($page)");

    if (page == 0) {
      final items = await ref.watch(cursorProvider(id, cursor: null).future);
      print("done with historyProvider($page) - first page");
      return items;
    }

    final lastPage = await ref.watch(
      historyProvider(id, page: page - 1).future,
    );
    if (lastPage.isEmpty) {
      print("done with historyProvider($page) - the previous page was empty!");
      return [];
    }

    final items = await ref.watch(cursorProvider(id, cursor: page).future);
    print("done with historyProvider($page)");
    return items;
  }
}

@riverpod
FutureOr<List<int>> cursor(Ref ref, int id, {required int? cursor}) async {
  print("executing cursorProvider($id, $cursor)");
  return Future.delayed(
    Duration(milliseconds: 1),
    () => List.generate(15, (index) => math.Random().nextInt(10)),
  );
}

Expected behavior

I'd expect the same behavior, with or without persisting values.

lucavenir avatar May 20 '25 14:05 lucavenir

Pretty sure that's not a loop, just an exponential amount of rebuild.

The first provider emits two updates (persist + initialized)

The second has 3 (persist + initial + rebuild from first provider)

Third has 4 (persist + initial + rebuild from 1st + rebuild from 2nd)

...

rrousselGit avatar May 20 '25 15:05 rrousselGit

Wait, what's that?

final items = await ref.watch(cursorProvider(id, cursor: page).future);

A provider is watching itself with the exact same args?

How is that expected to ever resolve? I'm surprised this isn't throwing.

You probably just want await future or to read state

rrousselGit avatar May 20 '25 15:05 rrousselGit

Pretty sure that's not a loop, just an exponential amount of rebuild.

You're right. That's probably the case, given how the logs unroll

A provider is watching itself with the exact same args?

I carefully re-read that, but no. There's historyProvider, which is the class-based provider that persists stuff, and there's cursorProvider, which simply does the heavy lifting (AKA network requests).

I'm sorry I confused you, and to be fair I can further simplify this. I just wanted to give context on how I've landed here. It's based on a discussion we had a month ago (or so) on Discord, about translating index-based pagination, to cursor-based ones.

lucavenir avatar May 20 '25 16:05 lucavenir

Hello there! To try and simplify things, I've rewrote the above example, so it's simpler and just one class. It's a simple Notifier with a mock method, nothing too fancy.

The main point is: this provider watches "its previous" value.

@riverpod
class History extends _$History with Persistable<List<int>, String, String> {
  static const delay = Duration(milliseconds: 1);
  static const pageSize = 10;

  @override
  FutureOr<List<int>> build(int id, {required int page}) async {
    print('built: $id, page: $page');
    final storage = await ref.watch(storageProvider.future);

    await persist(
      key: 'history-$id-$page',
      storage: storage,
      encode: (state) => convert.jsonEncode(state),
      decode: (encoded) {
        final decoded = convert.jsonDecode(encoded) as List<Object?>;
        return decoded.cast<int>();
      },
    );

    if (page == 0) {
      final items = await Future.delayed(delay, () => mock(0));
      return items;
    }

    final lastPage = await ref.watch(
      historyProvider(id, page: page - 1).future,
    );
    if (lastPage.isEmpty) return [];

    final items = await Future.delayed(delay, () => mock(lastPage.last));
    return items;
  }

  List<int> mock(int i) {
    return List.generate(
      pageSize,
      (index) => math.Random().nextInt(i + 10 * index),
    );
  }
}

My storageProvider just encapsulates riverpod_sqflite's APIs.

@riverpod
FutureOr<JsonSqFliteStorage> storage(Ref ref) async {
  final dir = await path_provider.getApplicationDocumentsDirectory();
  final dbPath = path.join(dir.path, 'db.db');
  final storage = await JsonSqFliteStorage.open(dbPath);
  ref
    ..onDispose(storage.close)
    ..keepAlive();

  return storage;
}

To replicate the bug, just ref.listen(historyProvider(99, page: 100), ...); in a home page of a example app 😄 You'll see this "print" loop:

Details
built: 99, page: 100
built: 99, page: 99
built: 99, page: 98
built: 99, page: 97
built: 99, page: 96
built: 99, page: 95
built: 99, page: 94
built: 99, page: 93
built: 99, page: 92
built: 99, page: 91
built: 99, page: 90
built: 99, page: 89
built: 99, page: 88
built: 99, page: 87
built: 99, page: 86
built: 99, page: 85
built: 99, page: 84
built: 99, page: 83
built: 99, page: 82
built: 99, page: 81
built: 99, page: 80
built: 99, page: 79
built: 99, page: 78
built: 99, page: 77
built: 99, page: 76
built: 99, page: 75
built: 99, page: 74
built: 99, page: 73
built: 99, page: 72
built: 99, page: 71
built: 99, page: 70
built: 99, page: 69
built: 99, page: 68
built: 99, page: 67
built: 99, page: 66
built: 99, page: 65
built: 99, page: 64
built: 99, page: 63
built: 99, page: 62
built: 99, page: 61
built: 99, page: 60
built: 99, page: 59
built: 99, page: 58
built: 99, page: 57
built: 99, page: 56
built: 99, page: 55
built: 99, page: 54
built: 99, page: 53
built: 99, page: 52
built: 99, page: 51
built: 99, page: 50
built: 99, page: 49
built: 99, page: 48
built: 99, page: 47
built: 99, page: 46
built: 99, page: 45
built: 99, page: 44
built: 99, page: 43
built: 99, page: 42
built: 99, page: 41
built: 99, page: 40
built: 99, page: 39
built: 99, page: 38
built: 99, page: 37
built: 99, page: 36
built: 99, page: 35
built: 99, page: 34
built: 99, page: 33
built: 99, page: 32
built: 99, page: 31
built: 99, page: 30
built: 99, page: 29
built: 99, page: 28
built: 99, page: 27
built: 99, page: 26
built: 99, page: 25
built: 99, page: 24
built: 99, page: 23
built: 99, page: 22
built: 99, page: 21
built: 99, page: 20
built: 99, page: 19
built: 99, page: 18
built: 99, page: 17
built: 99, page: 16
built: 99, page: 15
built: 99, page: 14
built: 99, page: 13
built: 99, page: 12
built: 99, page: 11
built: 99, page: 10
built: 99, page: 9
built: 99, page: 8
built: 99, page: 7
built: 99, page: 6
built: 99, page: 5
built: 99, page: 4
built: 99, page: 3
built: 99, page: 2
built: 99, page: 1
built: 99, page: 0
built: 99, page: 100
built: 99, page: 99
built: 99, page: 98
built: 99, page: 97
built: 99, page: 96
built: 99, page: 95
built: 99, page: 94
built: 99, page: 93
built: 99, page: 92
built: 99, page: 91
built: 99, page: 90
built: 99, page: 89
built: 99, page: 88
built: 99, page: 87
built: 99, page: 86
built: 99, page: 85
built: 99, page: 84
built: 99, page: 83
built: 99, page: 82
built: 99, page: 81
built: 99, page: 80
built: 99, page: 79
built: 99, page: 78
built: 99, page: 77
built: 99, page: 76
built: 99, page: 75
built: 99, page: 74
built: 99, page: 73
built: 99, page: 72
built: 99, page: 71
built: 99, page: 70
built: 99, page: 69
built: 99, page: 68
built: 99, page: 67
built: 99, page: 66
built: 99, page: 65
built: 99, page: 64
built: 99, page: 63
built: 99, page: 62
built: 99, page: 61
built: 99, page: 60
built: 99, page: 59
built: 99, page: 58
built: 99, page: 57
built: 99, page: 56
built: 99, page: 55
built: 99, page: 54
built: 99, page: 53
built: 99, page: 52
built: 99, page: 51
built: 99, page: 50
built: 99, page: 49
built: 99, page: 48
built: 99, page: 47
built: 99, page: 46
built: 99, page: 45
built: 99, page: 44
built: 99, page: 43
built: 99, page: 42
built: 99, page: 41
built: 99, page: 40
built: 99, page: 39
built: 99, page: 38
built: 99, page: 37
built: 99, page: 36
built: 99, page: 35
built: 99, page: 34
built: 99, page: 33
built: 99, page: 32
built: 99, page: 31
built: 99, page: 30
built: 99, page: 29
built: 99, page: 28
built: 99, page: 27
built: 99, page: 26
built: 99, page: 25
built: 99, page: 24
built: 99, page: 23
built: 99, page: 22
built: 99, page: 21
built: 99, page: 20
built: 99, page: 19
built: 99, page: 18
built: 99, page: 17
built: 99, page: 16
built: 99, page: 15
built: 99, page: 14
built: 99, page: 13
built: 99, page: 12
built: 99, page: 11
built: 99, page: 10
built: 99, page: 9
built: 99, page: 8
built: 99, page: 7
built: 99, page: 6
built: 99, page: 5
built: 99, page: 4
built: 99, page: 3
built: 99, page: 2
built: 99, page: 1
built: 99, page: 0
built: 99, page: 99
built: 99, page: 100
built: 99, page: 99
built: 99, page: 98
built: 99, page: 97
built: 99, page: 96
// ....

The immediate consequence is that I cannot use persist on a provider that watches itself as of now.


I've tried writing a self-enclosing test, but with no luck so far.

import 'package:flutter_riverpod/experimental/persist.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mocktail/mocktail.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';
import 'package:riverpod_persist_problem/history_provider.dart';
import 'package:riverpod_persist_problem/storage_provider.dart';
import 'package:riverpod_sqflite/riverpod_sqflite.dart';

void main() {
  setUpAll(() {
    registerFallbackValue(StorageOptions());
  });
  test("self-reading persistable provider should compute normally", () {
    final mock = StorageMock();
    const mockedData = "[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15]";
    when(
      () => mock.read(any()),
    ).thenAnswer((_) async => PersistedData(mockedData));
    when(
      () => mock.write(any(), any(), any()),
    ).thenAnswer((_) async => mockedData);
    final container = ProviderContainer.test(
      overrides: [storageProvider.overrideWith((ref) => mock)],
    );

    final reader = container.listen(
      historyProvider(99, page: 100).future,
      (previous, next) {},
    );

    expectLater(
      reader.read(),
      completion(equals([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15])),
    );
  });
}

class StorageMock<KeyT, EncodedT> extends Mock implements JsonSqFliteStorage {}

The above outputs:

00:00 +0: self-reading persistable provider should compute normally                                                                                     
built: 99, page: 100
built: 99, page: 99
00:00 +1: All tests passed!                                                                                                                             

Probably because I'm mocking "too much" of the storage logic, but I'm unsure how to mock "less" than that. Let me know what I could do to improve this report.

lucavenir avatar May 28 '25 15:05 lucavenir

Probably because I'm mocking "too much" of the storage logic, but I'm unsure how to mock "less" than that. Let me know what I could do to improve this report.

You generally shouldn't mock Storage. Instead use Storage.inMemory, and override your storage provider.

rrousselGit avatar Jun 02 '25 15:06 rrousselGit

Ok, done. Thank you for the inputs these days.

self-enclosing test
@Timeout(Duration(seconds: 1))
library;

import 'package:flutter_riverpod/experimental/persist.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:path/path.dart' as path;
import 'package:path_provider/path_provider.dart' as path_provider;
import 'package:riverpod_sqflite/riverpod_sqflite.dart';
import 'dart:convert' as convert;

class TestNotifier extends FamilyAsyncNotifier<List<int>, int>
    with Persistable<List<int>, String, String> {
  static const delay = Duration(milliseconds: 1);
  // try the following, and the test passes:
  // static const delay = Duration(microseconds: 1);
  static const pageSize = 10;

  @override
  FutureOr<List<int>> build(int page) async {
    final storage = await ref.watch(storageProvider.future);

    await persist(
      key: 'test-$page',
      storage: storage,
      encode: (state) => convert.jsonEncode(state),
      decode: (encoded) {
        final decoded = convert.jsonDecode(encoded) as List<Object?>;
        return decoded.cast<int>();
      },
    );

    if (page == 0) {
      final items = await Future.delayed(delay, () => mock(0));
      return items;
    }

    final lastPage = await ref.watch(testNotifierProvider(page - 1).future);
    if (lastPage.isEmpty) return [];

    final items = await Future.delayed(delay, () => mock(lastPage.last));
    return items;
  }

  List<int> mock(int i) {
    return List.generate(pageSize, (index) => i + index);
  }
}

final testNotifierProvider = AsyncNotifierProvider.autoDispose
    .family<TestNotifier, List<int>, int>(TestNotifier.new);

final storageProvider = FutureProvider<Storage<String, String>>((ref) async {
  final dir = await path_provider.getApplicationDocumentsDirectory();
  final dbPath = path.join(dir.path, 'db.db');
  final storage = await JsonSqFliteStorage.open(dbPath);
  ref
    ..onDispose(storage.close)
    ..keepAlive();

  return storage;
});

void main() {
  test("self-reading persistable provider should eventually compute", () async {
    const options = StorageOptions();
    final firstPageKey = 'test-0';
    final firstPageValue = '[0,1,2,3,4,5,6,7,8,9]';
    final storage = Storage<String, String>.inMemory();
    storage.write(firstPageKey, firstPageValue, options);
    final container = ProviderContainer.test(
      overrides: [storageProvider.overrideWith((ref) => storage)],
    );

    container.listen(testNotifierProvider(1000), (previous, next) {});

    // wait for the first page to read the persisted value
    await container.read(testNotifierProvider(0).future);
    // wait for the 100th page to compute
    await container.read(testNotifierProvider(1000).future);
    // the first page gets value from the network, others must recompute
    await container.pump();

    // test fails because of a timeout
    expectLater(container.read(testNotifierProvider(1000).future), completes);
  });
}

TL;DR: a notifier that had a previous persisted value, which updates with a network value, leads riverpod in a rebuild loop, or, rather, in an exponential amount of rebuilds. The test fails because of a timeout, which is set to 1.seconds here for convenience, but you can change it to whatever.

Note. Try lowering the delay parameter from 1.millisecond to 999.microseconds, and the test will pass. Note 2. Try lowering the family param from 1000 to 10 (I mean: in the second container.read), and the test will pass. It seems like that up until page: ~90, the test won't fail, aka no timeouts happen.

If the test checks out, and you're satisfied with it, we can remove need-triage.

lucavenir avatar Jun 05 '25 10:06 lucavenir