diff --git a/CHANGELOG.md b/CHANGELOG.md index 3de1d8f..6921f66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## Unreleased + +* Fix `SplashBuilder` so reactive errors only take over when they block bootstrap: + * initial reactive run failure (no previous value), or + * trigger-caused reactive refresh failure. +* Non-trigger runtime reactive failures after successful bootstrap no longer replace app content with splash error UI. +* Add tests for reactive error visibility rules. + ## 0.0.1 * Initial release diff --git a/README.md b/README.md index 37cd371..45652ec 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ reactiveTask: ReactiveTask( ), ``` -**Key behavior:** Only `trigger` changes show splash. Using `ref.watch()` in `run` keeps providers alive and re-runs silently without splash. +**Key behavior:** Only `trigger` changes show splash. Using `ref.watch()` in `run` keeps providers alive and re-runs silently without splash. If a non-trigger runtime refresh fails after a successful reactive run, Riverboot keeps rendering your app content instead of taking over with the splash error screen. ## Retry Support diff --git a/example/pubspec.lock b/example/pubspec.lock index bf0f437..dbe33ce 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -5,18 +5,18 @@ packages: dependency: transitive description: name: _fe_analyzer_shared - sha256: da0d9209ca76bde579f2da330aeb9df62b6319c834fa7baae052021b0462401f + sha256: "8d7ff3948166b8ec5da0fbb5962000926b8e02f2ed9b3e51d1738905fbd4c98d" url: "https://pub.dev" source: hosted - version: "85.0.0" + version: "93.0.0" analyzer: dependency: transitive description: name: analyzer - sha256: "974859dc0ff5f37bc4313244b3218c791810d03ab3470a579580279ba971a48d" + sha256: de7148ed2fcec579b19f122c1800933dfa028f6d9fd38a152b04b1516cec120b url: "https://pub.dev" source: hosted - version: "7.7.1" + version: "10.0.1" args: dependency: transitive description: @@ -45,10 +45,10 @@ packages: dependency: transitive description: name: characters - sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 + sha256: faf38497bda5ead2a8c7615f4f7939df04333478bf32e4173fcb06d428b5716b url: "https://pub.dev" source: hosted - version: "1.4.0" + version: "1.4.1" cli_config: dependency: transitive description: @@ -93,10 +93,10 @@ packages: dependency: transitive description: name: crypto - sha256: "1e445881f28f22d6140f181e07737b22f1e099a5e1ff94b0af2f9e4a463f4855" + sha256: c8ea0233063ba03258fbcf2ca4d6dadfefe14f02fab57702265467a19f27fadf url: "https://pub.dev" source: hosted - version: "3.0.6" + version: "3.0.7" cupertino_icons: dependency: "direct main" description: @@ -138,10 +138,10 @@ packages: dependency: "direct main" description: name: flutter_riverpod - sha256: "9e2d6907f12cc7d23a846847615941bddee8709bf2bfd274acdf5e80bcf22fde" + sha256: e2026c72738a925a60db30258ff1f29974e40716749f3c9850aabf34ffc1a14c url: "https://pub.dev" source: hosted - version: "3.0.3" + version: "3.2.1" flutter_test: dependency: "direct dev" description: flutter @@ -187,14 +187,6 @@ packages: url: "https://pub.dev" source: hosted version: "1.0.5" - js: - dependency: transitive - description: - name: js - sha256: "53385261521cc4a0c4658fd0ad07a7d14591cf8fc33abbceae306ddb974888dc" - url: "https://pub.dev" - source: hosted - version: "0.7.2" leak_tracker: dependency: transitive description: @@ -239,18 +231,18 @@ packages: dependency: transitive description: name: matcher - sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 + sha256: "12956d0ad8390bbcc63ca2e1469c0619946ccb52809807067a7020d57e647aa6" url: "https://pub.dev" source: hosted - version: "0.12.17" + version: "0.12.18" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec + sha256: "9c337007e82b1889149c82ed242ed1cb24a66044e30979c44912381e9be4c48b" url: "https://pub.dev" source: hosted - version: "0.11.1" + version: "0.13.0" meta: dependency: transitive description: @@ -295,10 +287,10 @@ packages: dependency: transitive description: name: pool - sha256: "20fe868b6314b322ea036ba325e6fc0711a22948856475e2c2b6306e8ab39c2a" + sha256: "978783255c543aa3586a1b3c21f6e9d720eb315376a915872c61ef8b5c20177d" url: "https://pub.dev" source: hosted - version: "1.5.1" + version: "1.5.2" pub_semver: dependency: transitive description: @@ -313,15 +305,15 @@ packages: path: ".." relative: true source: path - version: "0.0.2" + version: "0.1.2" riverpod: dependency: transitive description: name: riverpod - sha256: c406de02bff19d920b832bddfb8283548bfa05ce41c59afba57ce643e116aa59 + sha256: "8c22216be8ad3ef2b44af3a329693558c98eca7b8bd4ef495c92db0bba279f83" url: "https://pub.dev" source: hosted - version: "3.0.3" + version: "3.2.1" shelf: dependency: transitive description: @@ -379,10 +371,10 @@ packages: dependency: transitive description: name: source_span - sha256: "254ee5351d6cb365c859e20ee823c3bb479bf4a293c22d17a9f1bf144ce86f7c" + sha256: "56a02f1f4cd1a2d96303c0144c93bd6d909eea6bee6bf5a0e0b685edbd4c47ab" url: "https://pub.dev" source: hosted - version: "1.10.1" + version: "1.10.2" stack_trace: dependency: transitive description: @@ -427,26 +419,26 @@ packages: dependency: transitive description: name: test - sha256: "75906bf273541b676716d1ca7627a17e4c4070a3a16272b7a3dc7da3b9f3f6b7" + sha256: "54c516bbb7cee2754d327ad4fca637f78abfc3cbcc5ace83b3eda117e42cd71a" url: "https://pub.dev" source: hosted - version: "1.26.3" + version: "1.29.0" test_api: dependency: transitive description: name: test_api - sha256: ab2726c1a94d3176a45960b6234466ec367179b87dd74f1611adb1f3b5fb9d55 + sha256: "93167629bfc610f71560ab9312acdda4959de4df6fac7492c89ff0d3886f6636" url: "https://pub.dev" source: hosted - version: "0.7.7" + version: "0.7.9" test_core: dependency: transitive description: name: test_core - sha256: "0cc24b5ff94b38d2ae73e1eb43cc302b77964fbf67abad1e296025b78deb53d0" + sha256: "394f07d21f0f2255ec9e3989f21e54d3c7dc0e6e9dbce160e5a9c1a6be0e2943" url: "https://pub.dev" source: hosted - version: "0.6.12" + version: "0.6.15" typed_data: dependency: transitive description: @@ -475,10 +467,10 @@ packages: dependency: transitive description: name: watcher - sha256: "5bf046f41320ac97a469d506261797f35254fa61c641741ef32dacda98b7d39c" + sha256: "1398c9f081a753f9226febe8900fce8f7d0a67163334e1c94a2438339d79d635" url: "https://pub.dev" source: hosted - version: "1.1.3" + version: "1.2.1" web: dependency: transitive description: diff --git a/lib/src/bootstrap.dart b/lib/src/bootstrap.dart index b72302e..00e900b 100644 --- a/lib/src/bootstrap.dart +++ b/lib/src/bootstrap.dart @@ -23,15 +23,14 @@ class Riverboot { List overrides = const [], List? observers, - bool Function(Object, StackTrace)? onPlatformDispatchError, + bool Function(Object error, StackTrace stack)? onPlatformDispatchError, void Function(WidgetRef ref)? earlyEagerInitializer, void Function(Object error, StackTrace stack)? onError, }) async { final container = ProviderContainer( parent: parent, overrides: [ - if (splashConfig != null) - _splashConfigProvider.overrideWithValue(splashConfig), + if (splashConfig != null) _splashConfigProvider.overrideWithValue(splashConfig), ...overrides, ], observers: observers, diff --git a/lib/src/splash_builder.dart b/lib/src/splash_builder.dart index 4b43932..9f70fa9 100644 --- a/lib/src/splash_builder.dart +++ b/lib/src/splash_builder.dart @@ -1,5 +1,14 @@ part of 'src.dart'; +@visibleForTesting +bool shouldDisplayReactiveTaskError({ + required AsyncValue reactiveTaskRun, + required bool triggerCausedRefresh, +}) { + return reactiveTaskRun.hasError && + (triggerCausedRefresh || !reactiveTaskRun.hasValue); +} + /// A widget that shows a splash screen while tasks are loading. /// /// Place this in your [MaterialApp] or [CupertinoApp] builder function. @@ -36,7 +45,8 @@ class _SplashBuilderState extends ConsumerState { final reactiveTaskRun = ref.watch(_reactiveTaskRunProvider); // Clear trigger flag once reactive task completes (has value or error) - if ((reactiveTaskRun.hasValue || reactiveTaskRun.hasError) && _triggerCausedRefresh) { + if ((reactiveTaskRun.hasValue || reactiveTaskRun.hasError) && + _triggerCausedRefresh) { // Use Future.microtask to avoid modifying state during build Future.microtask(() { if (mounted) { @@ -60,11 +70,25 @@ class _SplashBuilderState extends ConsumerState { final isComplete = oneTimeComplete && reactiveComplete; - // Check for errors - prioritize one-time task errors + // Check for errors - prioritize one-time task errors. + // Reactive errors should take over only when they block boot: + // - initial reactive run failed (no previous successful value), or + // - trigger-caused refresh failed. + final showReactiveError = shouldDisplayReactiveTaskError( + reactiveTaskRun: reactiveTaskRun, + triggerCausedRefresh: _triggerCausedRefresh, + ); + final error = oneTimeTask.hasError - ? SplashTaskError(error: oneTimeTask.error!, stack: oneTimeTask.stackTrace!) - : reactiveTaskRun.hasError - ? SplashTaskError(error: reactiveTaskRun.error!, stack: reactiveTaskRun.stackTrace!) + ? SplashTaskError( + error: oneTimeTask.error!, + stack: oneTimeTask.stackTrace!, + ) + : showReactiveError + ? SplashTaskError( + error: reactiveTaskRun.error!, + stack: reactiveTaskRun.stackTrace!, + ) : null; if (error != null) { @@ -139,7 +163,8 @@ class _SplashTransition extends StatefulWidget { State<_SplashTransition> createState() => _SplashTransitionState(); } -class _SplashTransitionState extends State<_SplashTransition> with SingleTickerProviderStateMixin { +class _SplashTransitionState extends State<_SplashTransition> + with SingleTickerProviderStateMixin { late final AnimationController _controller; late final Animation _fadeAnimation; bool _showChild = false; @@ -148,8 +173,14 @@ class _SplashTransitionState extends State<_SplashTransition> with SingleTickerP @override void initState() { super.initState(); - _controller = AnimationController(duration: widget.config.fadeDuration, vsync: this); - _fadeAnimation = CurvedAnimation(parent: _controller, curve: Curves.easeOut); + _controller = AnimationController( + duration: widget.config.fadeDuration, + vsync: this, + ); + _fadeAnimation = CurvedAnimation( + parent: _controller, + curve: Curves.easeOut, + ); _controller.addStatusListener(_onAnimationEnd); if (widget.isComplete) { diff --git a/test/reactive_task_test.dart b/test/reactive_task_test.dart index 55c2ae3..a28e1f5 100644 --- a/test/reactive_task_test.dart +++ b/test/reactive_task_test.dart @@ -281,90 +281,42 @@ void main() { expect(find.text('Error type: StateError'), findsOneWidget); }); - testWidgets('error state does not show splash when trigger unchanged', (tester) async { - // This tests the fix: when reactive task errors without trigger change, - // it should go directly to error screen without flashing splash - var runCount = 0; - var splashBuildCount = 0; - var errorScreenShown = false; - - final triggerNotifier = ValueNotifier(0); - - final triggerProvider = Provider((ref) { - return triggerNotifier.value; - }); - - // Provider that will error on second run (without trigger change) - var shouldError = false; - - await tester.pumpWidget( - ProviderScope( - overrides: [ - splashConfigProvider.overrideWithValue( - SplashConfig( - splashBuilder: (error, retry) { - if (error != null) { - errorScreenShown = true; - return ElevatedButton( - onPressed: retry, - child: const Text('Error - Retry'), - ); - } - splashBuildCount++; - return const Text('Splash'); - }, - reactiveTask: ReactiveTask( - trigger: (ref) => ref.watch(triggerProvider), - run: (ref) async { - runCount++; - // Error if shouldError is true - if (shouldError) { - throw Exception('Intentional error'); - } - await Future.delayed(const Duration(milliseconds: 10)); - }, - ), - ), - ), - ], - child: const MaterialApp( - home: SplashBuilder(child: Text('Content')), - ), - ), + test( + 'runtime reactive error is hidden when previous value exists', + () { + final runtimeError = + const AsyncError( + 'runtime error', + StackTrace.empty, + ) + // ignore: invalid_use_of_internal_member + .copyWithPrevious(const AsyncData(null)); + + final shouldShow = shouldDisplayReactiveTaskError( + reactiveTaskRun: runtimeError, + triggerCausedRefresh: false, + ); + + expect(runtimeError.hasError, isTrue); + expect(runtimeError.hasValue, isTrue); + expect(shouldShow, isFalse); + }, + ); + + test('initial reactive error is shown when no previous value exists', () { + const initialError = AsyncError( + 'initial error', + StackTrace.empty, ); - // Initial load - splash shown, then content - await tester.pumpAndSettle(); - expect(find.text('Content'), findsOneWidget); - expect(runCount, 1); - final initialSplashCount = splashBuildCount; - - // Now trigger an error WITHOUT changing the trigger - // This simulates a dependency of run() causing a re-run that errors - final container = ProviderScope.containerOf( - tester.element(find.text('Content')), + final shouldShow = shouldDisplayReactiveTaskError( + reactiveTaskRun: initialError, + triggerCausedRefresh: false, ); - // Set error flag and invalidate run provider to simulate re-run - shouldError = true; - container.invalidate(reactiveTaskRunProvider); - - // Pump to process the error - await tester.pump(); - await tester.pump(); - await tester.pump(); - - // Error screen should be shown - expect(errorScreenShown, isTrue); - expect(find.text('Error - Retry'), findsOneWidget); - - // CRITICAL: Splash should NOT have been shown again - // (splashBuildCount should not have increased) - expect( - splashBuildCount, - initialSplashCount, - reason: 'Splash should not show when error occurs without trigger change', - ); + expect(initialError.hasError, isTrue); + expect(initialError.hasValue, isFalse); + expect(shouldShow, isTrue); }); test('parallel execution fails fast with eagerError', () async { @@ -482,24 +434,27 @@ void main() { expect(stopwatch.elapsed, greaterThanOrEqualTo(minimumDuration)); }); - test('zero tasks with zero minimum duration completes immediately', () async { - final container = ProviderContainer.test( - overrides: [ - splashConfigProvider.overrideWithValue( - SplashConfig( - splashBuilder: (_, _) => const SizedBox.shrink(), - minimumDuration: Duration.zero, - tasks: [], + test( + 'zero tasks with zero minimum duration completes immediately', + () async { + final container = ProviderContainer.test( + overrides: [ + splashConfigProvider.overrideWithValue( + SplashConfig( + splashBuilder: (_, _) => const SizedBox.shrink(), + minimumDuration: Duration.zero, + tasks: [], + ), ), - ), - ], - ); + ], + ); - final stopwatch = Stopwatch()..start(); - await container.read(splashTasksProvider.future); - stopwatch.stop(); + final stopwatch = Stopwatch()..start(); + await container.read(splashTasksProvider.future); + stopwatch.stop(); - expect(stopwatch.elapsed, lessThan(const Duration(milliseconds: 50))); - }); + expect(stopwatch.elapsed, lessThan(const Duration(milliseconds: 50))); + }, + ); }); }