Add error handling at top level

ubuntu · Jun 12, 2024 · 89dccdb · 89dccdb
1 parent 56cf72e
commit 89dccdb
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 122 deletions.
diff --git a/packages/app_center/lib/main.dart b/packages/app_center/lib/main.dart
@@ -7,6 +7,7 @@ import 'package:app_center/l10n.dart';
 import 'package:app_center/packagekit.dart';
 import 'package:app_center/ratings.dart';
 import 'package:app_center/snapd.dart';
+import 'package:app_center/src/providers/error_stream_provider.dart';
 import 'package:app_center/store.dart';
 import 'package:app_center_ratings_client/app_center_ratings_client.dart';
 import 'package:flutter/material.dart';
@@ -21,9 +22,9 @@ import 'package:ubuntu_service/ubuntu_service.dart';
 import 'package:xdg_directories/xdg_directories.dart' as xdg;
 import 'package:yaru/yaru.dart';
 
-Future<void> main(List<String> args) async {
-  await YaruWindowTitleBar.ensureInitialized();
+final log = Logger('main');
 
+Future<void> main(List<String> args) async {
   final binaryName = p.basename(Platform.resolvedExecutable);
   Logger.setup(
     path: p.join(
@@ -66,8 +67,23 @@ Future<void> main(List<String> args) async {
   registerService(PackageKitClient.new);
   registerService(PackageKitService.new,
       dispose: (service) => service.dispose());
+  registerService(
+    ErrorStreamController.new,
+    dispose: (controller) => controller.close(),
+  );
 
   await initDefaultLocale();
 
-  runApp(const ProviderScope(child: StoreApp()));
+  await runZonedGuarded(
+    () async {
+      await YaruWindowTitleBar.ensureInitialized();
+      runApp(const ProviderScope(child: StoreApp()));
+    },
+    (error, stackTrace) {
+      log.error('Error propagated to top-level', error, stackTrace);
+      if (error is Exception) {
+        getService<ErrorStreamController>().add(error);
+      }
+    },
+  );
 }
diff --git a/packages/app_center/lib/src/providers/error_provider.dart b/packages/app_center/lib/src/providers/error_provider.dart
diff --git a/packages/app_center/lib/src/providers/error_stream_provider.dart b/packages/app_center/lib/src/providers/error_stream_provider.dart
@@ -0,0 +1,11 @@
+import 'dart:async';
+
+import 'package:flutter_riverpod/flutter_riverpod.dart';
+import 'package:ubuntu_service/ubuntu_service.dart';
+
+/// Used to listen to incoming errors to show them to the user.
+final errorStreamProvider = StreamProvider<Exception>(
+  (ref) => getService<ErrorStreamController>().stream,
+);
+
+typedef ErrorStreamController = StreamController<Exception>;
diff --git a/packages/app_center/lib/src/store/store_app.dart b/packages/app_center/lib/src/store/store_app.dart
@@ -4,7 +4,7 @@ import 'package:app_center/l10n.dart';
 import 'package:app_center/layout.dart';
 import 'package:app_center/search.dart';
 import 'package:app_center/snapd.dart';
-import 'package:app_center/src/providers/error_provider.dart';
+import 'package:app_center/src/providers/error_stream_provider.dart';
 import 'package:app_center/src/store/store_navigator.dart';
 import 'package:app_center/src/store/store_observer.dart';
 import 'package:app_center/src/store/store_pages.dart';
@@ -37,26 +37,12 @@ class _StoreAppState extends ConsumerState<StoreApp> {
 
   NavigatorState get _navigator => _navigatorKey.currentState!;
 
-  Future<void> _showError(BuildContext context, SnapdException e) {
-    return showErrorDialog(
-      context: context,
-      title: e.kind ?? 'Unknown Snapd Exception',
-      message: e.message,
-    );
-  }
-
   @override
   Widget build(BuildContext context) {
     ref.listen(routeStreamProvider, (prev, next) {
       next.whenData((route) => _navigator.pushNamed(route));
     });
 
-    ref.listen(errorProvider, (_, error) {
-      if (error.hasValue && error.value is SnapdException) {
-        _showError(context, error.value as SnapdException);
-      }
-    });
-
     return CallbackShortcuts(
       bindings: <ShortcutActivator, VoidCallback>{
         LogicalKeySet(LogicalKeyboardKey.control, LogicalKeyboardKey.keyF): () {
@@ -74,78 +60,97 @@ class _StoreAppState extends ConsumerState<StoreApp> {
           localizationsDelegates: localizationsDelegates,
           navigatorKey: ref.watch(materialAppNavigatorKeyProvider),
           supportedLocales: supportedLocales,
-          home: YaruWindowTitleSetter(
-            child: Scaffold(
-              appBar: YaruWindowTitleBar(
-                title: ConstrainedBox(
-                  constraints: const BoxConstraints(maxWidth: kSearchBarWidth),
-                  child: SearchField(
-                    onSearch: (query) =>
-                        _navigator.pushAndRemoveSearch(query: query),
-                    onSnapSelected: (name) => _navigator.pushSnap(name: name),
-                    onDebSelected: (id) => _navigator.pushDeb(id: id),
-                    searchFocus: searchFocus,
-                  ),
-                ),
-              ),
-              body: YaruMasterDetailPage(
-                navigatorKey: _navigatorKey,
-                navigatorObservers: [StoreObserver(ref)],
-                initialRoute: ref.watch(initialRouteProvider),
-                controller: ref.watch(yaruPageControllerProvider),
-                tileBuilder: (context, index, selected, availableWidth) =>
-                    pages[index].tileBuilder(context, selected),
-                pageBuilder: (context, index) =>
-                    pages[index].pageBuilder(context),
-                layoutDelegate: const YaruMasterFixedPaneDelegate(
-                  paneWidth: kPaneWidth,
-                ),
-                breakpoint: 0, // always landscape
-                onGenerateRoute: (settings) =>
-                    switch (StoreRoutes.routeOf(settings)) {
-                  StoreRoutes.deb => MaterialPageRoute(
-                      settings: settings,
-                      builder: (_) => DebPage(
-                            id: StoreRoutes.debOf(settings)!,
-                          )),
-                  StoreRoutes.snap => MaterialPageRoute(
-                      settings: settings,
-                      builder: (_) => SnapPage(
-                        snapName: StoreRoutes.snapOf(settings)!,
-                      ),
-                    ),
-                  StoreRoutes.search => MaterialPageRoute(
-                      settings: settings,
-                      builder: (_) => SearchPage(
-                        query: StoreRoutes.queryOf(settings),
-                        category: StoreRoutes.categoryOf(settings),
-                      ),
-                    ),
-                  StoreRoutes.externalTools => MaterialPageRoute(
-                      settings: settings,
-                      builder: (_) => const ExternalTools(),
-                    ),
-                  _ => null,
-                },
-              ),
-            ),
+          home: _StoreAppHome(
+            navigatorKey: _navigatorKey,
+            searchFocus: searchFocus,
           ),
         ),
       ),
     );
   }
 }
 
-class YaruWindowTitleSetter extends StatelessWidget {
-  const YaruWindowTitleSetter({required this.child, super.key});
+class _StoreAppHome extends ConsumerWidget {
+  const _StoreAppHome({
+    required this.navigatorKey,
+    required this.searchFocus,
+  });
+
+  final GlobalKey<NavigatorState> navigatorKey;
+  final FocusNode searchFocus;
 
-  final Widget child;
+  NavigatorState get navigator => navigatorKey.currentState!;
+
+  Future<void> _showError(BuildContext context, SnapdException e) {
+    return showErrorDialog(
+      context: context,
+      title: e.kind ?? 'Unknown Snapd Exception',
+      message: e.message,
+    );
+  }
 
   @override
-  Widget build(BuildContext context) {
+  Widget build(BuildContext context, WidgetRef ref) {
     final l10n = AppLocalizations.of(context);
     YaruWindow.of(context).setTitle(l10n.appCenterLabel);
 
-    return child;
+    ref.listen(errorStreamProvider, (_, error) {
+      if (error.hasValue && error.value is SnapdException) {
+        _showError(context, error.value as SnapdException);
+      }
+    });
+
+    return Scaffold(
+      appBar: YaruWindowTitleBar(
+        title: ConstrainedBox(
+          constraints: const BoxConstraints(maxWidth: kSearchBarWidth),
+          child: SearchField(
+            onSearch: (query) => navigator.pushAndRemoveSearch(query: query),
+            onSnapSelected: (name) => navigator.pushSnap(name: name),
+            onDebSelected: (id) => navigator.pushDeb(id: id),
+            searchFocus: searchFocus,
+          ),
+        ),
+      ),
+      body: YaruMasterDetailPage(
+        navigatorKey: navigatorKey,
+        navigatorObservers: [StoreObserver(ref)],
+        initialRoute: ref.watch(initialRouteProvider),
+        controller: ref.watch(yaruPageControllerProvider),
+        tileBuilder: (context, index, selected, availableWidth) =>
+            pages[index].tileBuilder(context, selected),
+        pageBuilder: (context, index) => pages[index].pageBuilder(context),
+        layoutDelegate: const YaruMasterFixedPaneDelegate(
+          paneWidth: kPaneWidth,
+        ),
+        breakpoint: 0, // always landscape
+        onGenerateRoute: (settings) => switch (StoreRoutes.routeOf(settings)) {
+          StoreRoutes.deb => MaterialPageRoute(
+              settings: settings,
+              builder: (_) => DebPage(
+                id: StoreRoutes.debOf(settings)!,
+              ),
+            ),
+          StoreRoutes.snap => MaterialPageRoute(
+              settings: settings,
+              builder: (_) => SnapPage(
+                snapName: StoreRoutes.snapOf(settings)!,
+              ),
+            ),
+          StoreRoutes.search => MaterialPageRoute(
+              settings: settings,
+              builder: (_) => SearchPage(
+                query: StoreRoutes.queryOf(settings),
+                category: StoreRoutes.categoryOf(settings),
+              ),
+            ),
+          StoreRoutes.externalTools => MaterialPageRoute(
+              settings: settings,
+              builder: (_) => const ExternalTools(),
+            ),
+          _ => null,
+        },
+      ),
+    );
   }
 }
diff --git a/packages/app_center/test/snap_page_test.dart b/packages/app_center/test/snap_page_test.dart
@@ -358,34 +358,5 @@ void main() {
     expect(find.text(snapRating.ratingsBand.localize(l10n)), findsNothing);
   });
 
-  testWidgets('error dialog', (tester) async {
-    final snapLauncher = createMockSnapLauncher(isLaunchable: true);
-    final updatesModel = createMockUpdatesModel();
-    final ratingsModel = createMockRatingsModel();
-    final container = createContainer(
-      overrides: [
-        launchProvider.overrideWith((ref, arg) => snapLauncher),
-        updatesModelProvider.overrideWith((ref) => updatesModel),
-        ratingsModelProvider.overrideWith((ref, arg) => ratingsModel),
-      ],
-    );
-
-    await tester.pumpApp(
-      (_) => UncontrolledProviderScope(
-        container: container,
-        child: SnapPage(snapName: storeSnap.name),
-      ),
-    );
-    container.read(snapModelProvider(storeSnap.name).notifier).state =
-        AsyncError(
-      SnapdException(message: 'error message', kind: 'error kind'),
-      StackTrace.current,
-    );
-    await tester.pumpAndSettle();
-
-    expect(find.text('error message'), findsOneWidget);
-    expect(find.text('error kind'), findsOneWidget);
-  });
-
   // TODO: test loading states with snap change in progress
 }
diff --git a/packages/app_center/test/store_app_test.dart b/packages/app_center/test/store_app_test.dart
@@ -1,10 +1,15 @@
+import 'dart:async';
+
 import 'package:app_center/ratings.dart';
 import 'package:app_center/snapd.dart';
+import 'package:app_center/src/providers/error_stream_provider.dart';
 import 'package:app_center/src/store/store_app.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:gtk/gtk.dart';
+import 'package:mockito/mockito.dart';
+import 'package:snapd/snapd.dart';
 import 'package:ubuntu_service/ubuntu_service.dart';
 import 'package:yaru/yaru.dart';
 
@@ -14,7 +19,8 @@ void main() {
   group('updates badge', () {
     testWidgets('no updates available', (tester) async {
       registerMockService<GtkApplicationNotifier>(
-          createMockGtkApplicationNotifier());
+        createMockGtkApplicationNotifier(),
+      );
       registerMockService<RatingsService>(createMockRatingsService());
       await tester.pumpApp(
         (_) => ProviderScope(
@@ -35,7 +41,8 @@ void main() {
 
     testWidgets('updates available', (tester) async {
       registerMockService<GtkApplicationNotifier>(
-          createMockGtkApplicationNotifier());
+        createMockGtkApplicationNotifier(),
+      );
       registerMockService<RatingsService>(createMockRatingsService());
       await tester.pumpApp(
         (_) => ProviderScope(
@@ -56,4 +63,63 @@ void main() {
       expect((tester.widget<Badge>(badge).label! as Text).data, equals('2'));
     });
   });
+
+  group('error handling', () {
+    testWidgets(
+      'errorStreamProvider receives exception when thrown',
+      (tester) async {
+        final snapdService = registerMockSnapdService();
+        registerService<ErrorStreamController>(ErrorStreamController.new);
+
+        final exception =
+            SnapdException(kind: 'error kind', message: 'error message');
+        when(snapdService.getSnap(any)).thenThrow(exception);
+
+        final container = createContainer();
+        unawaited(
+          runZonedGuarded(
+            () async {
+              await tester.pumpApp(
+                (_) => UncontrolledProviderScope(
+                  container: container,
+                  child: const StoreApp(),
+                ),
+              );
+              await container.read(snapModelProvider('snapName').future);
+            },
+            (error, stackTrace) {
+              if (error is Exception) {
+                getService<ErrorStreamController>().add(error);
+              }
+            },
+          ),
+        );
+
+        await expectLater(
+          container.read(errorStreamProvider.future).asStream(),
+          emits(exception),
+        );
+      },
+    );
+
+    testWidgets('showing error from error stream', (tester) async {
+      registerMockSnapdService();
+      await tester.pumpApp(
+        (_) => ProviderScope(
+          overrides: [
+            errorStreamProvider.overrideWith(
+              (ref) => Stream.value(
+                SnapdException(message: 'error message', kind: 'error kind'),
+              ),
+            ),
+          ],
+          child: const StoreApp(),
+        ),
+      );
+      await tester.pump();
+
+      expect(find.text('error message'), findsOneWidget);
+      expect(find.text('error kind'), findsOneWidget);
+    });
+  });
 }