Fix ProviderObserver.didUpdateProvider being called with an incorre…

…ct "provider" parameter when the provider is overridden. (#3125)

.github/workflows/changelog.yml

@@ -9,7 +9,7 @@ on:
 
 
 jobs:
-  build:
+  changelog:
     runs-on: ubuntu-latest
 
     steps:

.github/workflows/check_generation.yml

@@ -12,7 +12,7 @@ on:
       - "**.md"
 
 jobs:
-  build:
+  check_generation:
     runs-on: ubuntu-latest
 
     steps:

.github/workflows/riverpod_lint.yml

@@ -15,7 +15,7 @@ on:
     - cron: "0 10 * * *"
 
 jobs:
-  build:
+  riverpod_lint:
     runs-on: ubuntu-latest
 
     defaults:

packages/riverpod/CHANGELOG.md

@@ -1,3 +1,8 @@
+## Unreleased patch
+
+- Fix `ProviderObserver.didUpdateProvider` being called with an incorrect
+  "provider" parameter when the provider is overridden.
+
 ## 2.4.6 - 2023-11-13
 
 - Exceptions in asynchronous providers are now correctly received

packages/riverpod/lib/src/framework/element.dart

@@ -67,7 +67,7 @@ abstract class ProviderElementBase<State> implements Ref<State>, Node {
   /// The [ProviderContainer] that owns this [ProviderElementBase].
   @override
   ProviderContainer get container => _container;
-  late ProviderContainer _container;
+  late final ProviderContainer _container;
 
   /// Whether this [ProviderElementBase] is currently listened to or not.
   ///
@@ -573,7 +573,7 @@ The provider ${_debugCurrentlyBuildingElement!.origin} modified $origin while bu
     for (final observer in _container.observers) {
       runQuaternaryGuarded(
         observer.didUpdateProvider,
-        provider,
+        origin,
         previousState,
         newState.stateOrNull,
         _container,
@@ -586,7 +586,7 @@ The provider ${_debugCurrentlyBuildingElement!.origin} modified $origin while bu
         error: (newState) {
           runQuaternaryGuarded(
             observer.providerDidFail,
-            provider,
+            origin,
             newState.error,
             newState.stackTrace,
             _container,

packages/riverpod/test/framework/provider_observer_test.dart

@@ -69,49 +69,48 @@ void main() {
         verifyNever(observer.providerDidFail(any, any, any, any));
       });
 
-      // test(
-      //     'on scoped ProviderContainer, applies both child and ancestors observers',
-      //     () {
-      //   final provider = StateNotifierProvider<StateController<int>, int>(
-      //     (ref) => StateController(0),
-      //   );
-      //   final observer = ObserverMock();
-      //   final observer2 = ObserverMock();
-      //   final observer3 = ObserverMock();
-      //   final root = createContainer(observers: [observer]);
-      //   final mid = createContainer(
-      //     parent: root,
-      //     observers: [observer2],
-      //   );
-      //   final child = createContainer(
-      //     parent: mid,
-      //     overrides: [provider.overrideWithValue(StateController(42))],
-      //     observers: [observer3],
-      //   );
-
-      //   expect(child.read(provider), 42);
-      //   expect(mid.read(provider), 0);
-
-      //   clearInteractions(observer3);
-      //   clearInteractions(observer2);
-      //   clearInteractions(observer);
-
-      //   child.read(provider.notifier).state++;
-
-      //   verifyInOrder([
-      //     observer3.didUpdateProvider(provider, 42, 43, child),
-      //     observer2.didUpdateProvider(provider, 42, 43, child),
-      //     observer.didUpdateProvider(provider, 42, 43, child),
-      //   ]);
-
-      //   mid.read(provider.notifier).state++;
-
-      //   verify(observer.didUpdateProvider(provider, 0, 1, root)).called(1);
-
-      //   verifyNoMoreInteractions(observer3);
-      //   verifyNoMoreInteractions(observer2);
-      //   verifyNoMoreInteractions(observer);
-      // });
+      test(
+          'on scoped ProviderContainer, applies both child and ancestors observers',
+          () {
+        final provider = StateNotifierProvider<StateController<int>, int>(
+          (ref) => StateController(0),
+        );
+        final observer = ObserverMock('a');
+        final observer2 = ObserverMock('b');
+        final observer3 = ObserverMock('c');
+
+        final root = createContainer(observers: [observer]);
+        final mid = createContainer(
+          parent: root,
+          observers: [observer2],
+        );
+        final child = createContainer(
+          parent: mid,
+          overrides: [provider.overrideWith((ref) => StateController(42))],
+          observers: [observer3],
+        );
+
+        expect(child.read(provider), 42);
+        expect(mid.read(provider), 0);
+
+        clearInteractions(observer3);
+        clearInteractions(observer2);
+        clearInteractions(observer);
+
+        expect(child.read(provider.notifier).state++, 42);
+
+        verify(observer.didUpdateProvider(provider, 42, 43, child)).called(1);
+        verify(observer2.didUpdateProvider(provider, 42, 43, child)).called(1);
+        verify(observer3.didUpdateProvider(provider, 42, 43, child)).called(1);
+
+        mid.read(provider.notifier).state++;
+
+        verify(observer.didUpdateProvider(provider, 0, 1, root)).called(1);
+
+        verifyNoMoreInteractions(observer3);
+        verifyNoMoreInteractions(observer2);
+        verifyNoMoreInteractions(observer);
+      });
 
       test('handles computed provider update', () async {
         final observer = ObserverMock();
@@ -150,84 +149,76 @@ void main() {
         ).called(1);
       });
 
-      // test('didUpdateProviders', () {
-      //   final observer = ObserverMock();
-      //   final observer2 = ObserverMock();
-      //   final provider = StateNotifierProvider<Counter, int>((_) => Counter());
-      //   final counter = Counter();
-      //   final container = createContainer(
-      //     overrides: [
-      //       provider.overrideWithValue(counter),
-      //     ],
-      //     observers: [observer, observer2],
-      //   );
-      //   final listener = Listener<int>();
-
-      //   container.listen(provider, listener, fireImmediately: true);
-
-      //   verify(listener(null, 0)).called(1);
-      //   verifyNoMoreInteractions(listener);
-      //   verifyInOrder([
-      //     observer.didAddProvider(
-      //       provider.notifier,
-      //       counter,
-      //       container,
-      //     ),
-      //     observer2.didAddProvider(provider.notifier, counter, container),
-      //     observer.didAddProvider(provider, 0, container),
-      //     observer2.didAddProvider(provider, 0, container),
-      //   ]);
-      //   verifyNoMoreInteractions(observer);
-      //   verifyNoMoreInteractions(observer2);
-
-      //   counter.increment();
-
-      //   verifyInOrder([
-      //     listener(0, 1),
-      //     observer.didUpdateProvider(provider, 0, 1, container),
-      //     observer2.didUpdateProvider(provider, 0, 1, container),
-      //   ]);
-      //   verifyNoMoreInteractions(listener);
-      //   verifyNoMoreInteractions(observer);
-      //   verifyNoMoreInteractions(observer2);
-      // });
-
-      // test('guards didUpdateProviders', () {
-      //   final observer = ObserverMock();
-      //   when(observer.didUpdateProvider(any, any, any, any))
-      //       .thenThrow('error1');
-      //   final observer2 = ObserverMock();
-      //   when(observer2.didUpdateProvider(any, any, any, any))
-      //       .thenThrow('error2');
-      //   final observer3 = ObserverMock();
-      //   final provider = StateNotifierProvider<Counter, int>((_) => Counter());
-      //   final counter = Counter();
-      //   final container = createContainer(
-      //     overrides: [
-      //       provider.overrideWithValue(counter),
-      //     ],
-      //     observers: [observer, observer2, observer3],
-      //   );
-
-      //   container.read(provider);
-
-      //   clearInteractions(observer);
-      //   clearInteractions(observer2);
-      //   clearInteractions(observer3);
-
-      //   final errors = <Object>[];
-      //   runZonedGuarded(counter.increment, (err, stack) => errors.add(err));
-
-      //   expect(errors, ['error1', 'error2']);
-      //   verifyInOrder([
-      //     observer.didUpdateProvider(provider, 0, 1, container),
-      //     observer2.didUpdateProvider(provider, 0, 1, container),
-      //     observer3.didUpdateProvider(provider, 0, 1, container),
-      //   ]);
-      //   verifyNoMoreInteractions(observer);
-      //   verifyNoMoreInteractions(observer2);
-      //   verifyNoMoreInteractions(observer3);
-      // });
+      test('didUpdateProviders', () {
+        final observer = ObserverMock('a');
+        final observer2 = ObserverMock('b');
+        final provider = StateNotifierProvider<Counter, int>((_) => Counter());
+        final counter = Counter();
+        final container = createContainer(
+          overrides: [
+            provider.overrideWith((ref) => counter),
+          ],
+          observers: [observer, observer2],
+        );
+        final listener = Listener<int>();
+
+        container.listen(provider, listener.call, fireImmediately: true);
+
+        verify(listener(null, 0)).called(1);
+        verifyNoMoreInteractions(listener);
+        verifyInOrder([
+          observer.didAddProvider(provider, 0, container),
+          observer2.didAddProvider(provider, 0, container),
+        ]);
+        verifyNoMoreInteractions(observer);
+        verifyNoMoreInteractions(observer2);
+
+        counter.increment();
+
+        verifyInOrder([
+          listener(0, 1),
+          observer.didUpdateProvider(provider, 0, 1, container),
+          observer2.didUpdateProvider(provider, 0, 1, container),
+        ]);
+        verifyNoMoreInteractions(listener);
+        verifyNoMoreInteractions(observer);
+        verifyNoMoreInteractions(observer2);
+      });
+
+      test('guards didUpdateProviders', () {
+        final observer = ObserverMock();
+        when(observer.didUpdateProvider(any, any, any, any))
+            .thenThrow('error1');
+        final observer2 = ObserverMock();
+        when(observer2.didUpdateProvider(any, any, any, any))
+            .thenThrow('error2');
+        final observer3 = ObserverMock();
+        final provider = StateNotifierProvider<Counter, int>((_) => Counter());
+        final counter = Counter();
+        final container = createContainer(
+          overrides: [provider.overrideWith((ref) => counter)],
+          observers: [observer, observer2, observer3],
+        );
+
+        container.read(provider);
+
+        clearInteractions(observer);
+        clearInteractions(observer2);
+        clearInteractions(observer3);
+
+        final errors = <Object>[];
+        runZonedGuarded(counter.increment, (err, stack) => errors.add(err));
+
+        expect(errors, ['error1', 'error2']);
+        verifyInOrder([
+          observer.didUpdateProvider(provider, 0, 1, container),
+          observer2.didUpdateProvider(provider, 0, 1, container),
+          observer3.didUpdateProvider(provider, 0, 1, container),
+        ]);
+        verifyNoMoreInteractions(observer);
+        verifyNoMoreInteractions(observer2);
+        verifyNoMoreInteractions(observer3);
+      });
 
       test("Computed don't call didUpdateProviders when value doesn't change",
           () async {
@@ -288,6 +279,69 @@ void main() {
     });
 
     group('providerDidFail', () {
+      test(
+          'on scoped ProviderContainer, applies both child and ancestors observers',
+          () async {
+        final dep = StateProvider((ref) => 0);
+        final provider = StateNotifierProvider<StateController<int>, int>(
+          (ref) => StateController(0),
+        );
+        final observer = ObserverMock('a');
+        final observer2 = ObserverMock('b');
+        final observer3 = ObserverMock('c');
+
+        final root = createContainer(observers: [observer]);
+        final mid = createContainer(
+          parent: root,
+          observers: [observer2],
+        );
+        final child = createContainer(
+          parent: mid,
+          overrides: [
+            provider.overrideWith((ref) {
+              if (ref.watch(dep) != 0) {
+                Error.throwWithStackTrace('error', StackTrace.empty);
+              }
+              return StateController(42);
+            }),
+          ],
+          observers: [observer3],
+        );
+
+        child.listen(provider, (_, __) {}, onError: (_, __) {});
+
+        expect(child.read(provider), 42);
+        expect(mid.read(provider), 0);
+
+        clearInteractions(observer3);
+        clearInteractions(observer2);
+        clearInteractions(observer);
+
+        child.read(dep.notifier).state++;
+        await child.pump();
+
+        verifyInOrder([
+          observer.didDisposeProvider(provider, child),
+          observer.didUpdateProvider(dep, 0, 1, root),
+          observer.didUpdateProvider(provider, 42, null, child),
+          observer.providerDidFail(provider, 'error', StackTrace.empty, child),
+        ]);
+        verifyInOrder([
+          observer2.didDisposeProvider(provider, child),
+          observer2.didUpdateProvider(provider, 42, null, child),
+          observer2.providerDidFail(provider, 'error', StackTrace.empty, child),
+        ]);
+        verifyInOrder([
+          observer3.didDisposeProvider(provider, child),
+          observer3.didUpdateProvider(provider, 42, null, child),
+          observer3.providerDidFail(provider, 'error', StackTrace.empty, child),
+        ]);
+
+        verifyNoMoreInteractions(observer3);
+        verifyNoMoreInteractions(observer2);
+        verifyNoMoreInteractions(observer);
+      });
+
       test('is called when FutureProvider emits an error', () async {
         final observer = ObserverMock();
         final container = createContainer(observers: [observer]);

packages/riverpod/test/utils.dart

@@ -176,6 +176,15 @@ class _EqualsIgnoringHashCodes extends Matcher {
 }
 
 class ObserverMock extends Mock implements ProviderObserver {
+  ObserverMock([this.label]);
+
+  final String? label;
+
+  @override
+  String toString() {
+    return label ?? super.toString();
+  }
+
   @override
   void didDisposeProvider(
     ProviderBase<Object?>? provider,