FutureProvider/AsyncNotifier loses previous value when its rebuild resulting in AsyncError

[AhmedLSayed9]

Describe the bug

When FutureProvider/AsyncNotifier rebuilds because of Ref.watch/Ref.Refresh and resulting in AsyncError, It loses its previous data (value will be null).

This will break the intended behavior of skipError: true

To Reproduce

final throwErrorModifier = StateProvider((ref) => false);

final testProvider = FutureProvider.autoDispose<String>((ref) async {
  await Future.delayed(const Duration(seconds: 5));
  if (ref.watch(throwErrorModifier)) {
    throw Exception();
  }
  return 'data';
});

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

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final asyncValue = ref.watch(testProvider);
    
    return Scaffold(
      appBar: AppBar(
        title: const Text('Riverpod example'),
      ),
      body: Center(
        child: Column(
          mainAxisSize: MainAxisSize.min,
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            asyncValue.when(
              skipLoadingOnReload: true,
              skipLoadingOnRefresh: true,
              skipError: true,
              data: (data) => Text(data),
              loading: () => Text('loading'),
              error: (err, _) => Text('error'),
            ),
            ElevatedButton(
              onPressed: () {
                ref.read(throwErrorModifier.notifier).state = true;
              },
              child: Text('throw error'),
            ),
          ],
        ),
      ),
    );
  }
}

Expected behavior

When testProvider rebuilds, it'll keep showing the prev data while loading.
but when AsyncError triggers, it'll lose the prev data and will show error instead.

Expected behavior is to keep showing prev data as skipError is set to true

[roubachof]

I can see the same behavior.
Transitioning from AsyncData to AsyncLoading is working nicely and keep the value, (AsyncLoading has the previous value).
But the transition from AsyncLoading with data to AsyncError seems to lose the value, resulting in an incoherent state.

If it helps, I noticed that copyWithPrevious in the AsyncError<T> class is never called as opposed in the AsyncLoading class.

[roubachof]

Looking at the source, in my case which is an asyncNotifier, I think the copyWithPrevious method from the AsyncError is never called in the framework (well at least in the riverpod package).
There are three references when I look at usage, one is regarding stream provider, which I am not using in my PoC.
Two others are:

• common.dart line 23:

AsyncLoading<T>()
            .copyWithPrevious(previous, isRefresh: !shouldClearPreviousState),

• base.dart line 169:

set state(AsyncValue<T> newState) {
    // TODO assert Notifier isn't disposed
    newState.when(
      error: _errorTransition,
      loading: _loadingTransition,
      data: _dataTransition,
    );

    if (newState.isLoading) {
      setState(newState.copyWithPrevious(requireState, isRefresh: false));
    } else {
      setState(newState);
    }
  }

Well it's for manual set state. I still tried to put breakpoint at the 2 setState, but no hit...

So I guess a call to copyWithPrevious on the AsyncError is missing at some point...

[AhmedLSayed9]

@roubachof Thanks for clarifying! That's what I was thinking too