Fix provider bug that prevented to access games offline (#2519)

* Fix game provider bug

Closes #2473

* Make http error detection less specific

Author: veloce

lib/src/model/analysis/analysis_controller.dart

@@ -171,7 +171,7 @@ class AnalysisController extends AsyncNotifier<AnalysisState>
     switch (options) {
       case ArchivedGame(:final gameId):
         {
-          archivedGame = await ref.read(archivedGameProvider(gameId).future);
+          archivedGame = await ref.watch(archivedGameProvider(gameId).future);
           _variant = archivedGame!.meta.variant;
           if (!_variant.isReadSupported) {
             throw UnsupportedVariantException(_variant, gameId);

lib/src/model/analysis/retro_controller.dart

@@ -109,7 +109,7 @@ class RetroController extends AsyncNotifier<RetroState> with EngineEvaluationMix
 
     socketClient = ref.watch(socketPoolProvider).open(AnalysisController.socketUri);
 
-    _game = await ref.read(archivedGameProvider(options.id).future);
+    _game = await ref.watch(archivedGameProvider(options.id).future);
 
     if (engineSupportedVariants.contains(_game.meta.variant) == false) {
       throw Exception('Variant ${_game.meta.variant} is not supported for retro mode');

lib/src/model/game/game_repository.dart

@@ -6,6 +6,7 @@ import 'package:lichess_mobile/src/model/common/id.dart';
 import 'package:lichess_mobile/src/model/common/perf.dart';
 import 'package:lichess_mobile/src/model/game/exported_game.dart';
 import 'package:lichess_mobile/src/model/game/game_filter.dart';
+import 'package:lichess_mobile/src/model/game/game_storage.dart';
 import 'package:lichess_mobile/src/model/game/playable_game.dart';
 import 'package:lichess_mobile/src/network/aggregator.dart';
 import 'package:lichess_mobile/src/network/http.dart';
@@ -14,28 +15,42 @@ import 'package:lichess_mobile/src/network/http.dart';
 final gameRepositoryProvider = Provider<GameRepository>((ref) {
   final client = ref.watch(lichessClientProvider);
   final aggregator = ref.watch(aggregatorProvider);
-  return GameRepository(client, aggregator);
+  final gameStorage = ref.watch(gameStorageProvider.future);
+  return GameRepository(client, aggregator, gameStorage);
 }, name: 'GameRepositoryProvider');
 
 class GameRepository {
-  const GameRepository(this.client, this.aggregator);
+  const GameRepository(this.client, this.aggregator, this.storage);
 
   final LichessClient client;
   final Aggregator aggregator;
+  final Future<GameStorage> storage;
 
-  Future<ExportedGame> getGame(GameId id, {bool withBookmarked = false}) {
-    return client.readJson(
-      Uri(
-        path: '/game/export/$id',
-        queryParameters: {
-          'clocks': '1',
-          'accuracy': '1',
-          if (withBookmarked) 'withBookmarked': '1',
-        },
-      ),
-      headers: {'Accept': 'application/json'},
-      mapper: (json) => ExportedGame.fromServerJson(json, withBookmarked: withBookmarked),
-    );
+  /// Fetches a game from lichess API, or from local storage if there is no connectivity.
+  Future<ExportedGame> getGame(GameId id, {bool withBookmarked = false}) async {
+    try {
+      return await client.readJson(
+        Uri(
+          path: '/game/export/$id',
+          queryParameters: {
+            'clocks': '1',
+            'accuracy': '1',
+            if (withBookmarked) 'withBookmarked': '1',
+          },
+        ),
+        headers: {'Accept': 'application/json'},
+        mapper: (json) => ExportedGame.fromServerJson(json, withBookmarked: withBookmarked),
+      );
+    } on ServerException {
+      rethrow;
+      // Catches other exceptions like no connectivity
+    } on Exception {
+      final storedGame = await (await storage).fetch(gameId: id);
+      if (storedGame != null) {
+        return storedGame;
+      }
+      throw Exception('Game $id cannot be found in local storage.');
+    }
   }
 
   Future<void> requestServerAnalysis(GameId id) async {

lib/src/model/game/game_repository_providers.dart

@@ -4,27 +4,13 @@ import 'package:lichess_mobile/src/model/auth/auth_controller.dart';
 import 'package:lichess_mobile/src/model/common/id.dart';
 import 'package:lichess_mobile/src/model/game/exported_game.dart';
 import 'package:lichess_mobile/src/model/game/game_repository.dart';
-import 'package:lichess_mobile/src/model/game/game_storage.dart';
 
-/// Fetches a game from the server or from the local storage if not available online.
 final archivedGameProvider = FutureProvider.autoDispose.family<ExportedGame, GameId>((
   Ref ref,
   GameId id,
-) async {
-  ExportedGame game;
-  try {
-    final isLoggedIn = ref.watch(isLoggedInProvider);
-    game = await ref.read(gameRepositoryProvider).getGame(id, withBookmarked: isLoggedIn);
-  } catch (_) {
-    final gameStorage = await ref.watch(gameStorageProvider.future);
-    final storedGame = await gameStorage.fetch(gameId: id);
-    if (storedGame != null) {
-      game = storedGame;
-    } else {
-      throw Exception('Game $id not found in local storage.');
-    }
-  }
-  return game;
+) {
+  final isLoggedIn = ref.watch(isLoggedInProvider);
+  return ref.read(gameRepositoryProvider).getGame(id, withBookmarked: isLoggedIn);
 }, name: 'ArchivedGameProvider');
 
 final gamesByIdProvider = FutureProvider.autoDispose.family<IList<LightExportedGame>, ISet<GameId>>(

lib/src/network/http.dart

@@ -840,8 +840,8 @@ extension ClientRefExtension on Ref {
   ///
   /// This is primarily used for caching network requests in a [FutureProvider].
   ///
-  /// If [fn] throws with a [SocketException], the provider is not kept alive, this
-  /// allows to retry the request later.
+  /// If [fn] throws with a [ServerException], the provider is kept alive as we don't want to retry
+  /// server errors immediately.
   Future<U> withClientCacheFor<U>(Future<U> Function(LichessClient) fn, Duration duration) async {
     final link = keepAlive();
     final timer = Timer(duration, link.close);
@@ -851,10 +851,9 @@ extension ClientRefExtension on Ref {
     });
     try {
       return await fn(client);
-    } on SocketException catch (_) {
-      link.close();
+    } on ServerException {
       rethrow;
-    } on ClientException catch (_) {
+    } on Exception {
       link.close();
       rethrow;
     }
@@ -864,8 +863,8 @@ extension ClientRefExtension on Ref {
   ///
   /// This is primarily used for caching network requests in a [FutureProvider].
   ///
-  /// If [fn] throws with a [SocketException], the provider is not kept alive, this
-  /// allows to retry the request later.
+  /// If [fn] throws with a [ServerException], the provider is kept alive as we don't want to retry
+  /// server errors immediately.
   Future<U> withAggregatorCacheFor<U>(
     Future<U> Function(LichessClient, Aggregator) fn,
     Duration duration,
@@ -879,10 +878,9 @@ extension ClientRefExtension on Ref {
     });
     try {
       return await fn(client, aggregator);
-    } on SocketException catch (_) {
-      link.close();
+    } on ServerException {
       rethrow;
-    } on ClientException catch (_) {
+    } on Exception {
       link.close();
       rethrow;
     }

lib/src/view/puzzle/puzzle_screen.dart

@@ -11,7 +11,7 @@ import 'package:lichess_mobile/src/model/analysis/analysis_controller.dart';
 import 'package:lichess_mobile/src/model/auth/auth_controller.dart';
 import 'package:lichess_mobile/src/model/common/chess.dart';
 import 'package:lichess_mobile/src/model/common/id.dart';
-import 'package:lichess_mobile/src/model/game/game_repository_providers.dart';
+import 'package:lichess_mobile/src/model/game/game_repository.dart';
 import 'package:lichess_mobile/src/model/puzzle/puzzle.dart';
 import 'package:lichess_mobile/src/model/puzzle/puzzle_angle.dart';
 import 'package:lichess_mobile/src/model/puzzle/puzzle_controller.dart';
@@ -45,6 +45,7 @@ import 'package:lichess_mobile/src/widgets/adaptive_choice_picker.dart';
 import 'package:lichess_mobile/src/widgets/board.dart';
 import 'package:lichess_mobile/src/widgets/bottom_bar.dart';
 import 'package:lichess_mobile/src/widgets/buttons.dart';
+import 'package:lichess_mobile/src/widgets/feedback.dart';
 import 'package:lichess_mobile/src/widgets/list.dart';
 import 'package:lichess_mobile/src/widgets/pgn.dart';
 import 'package:lichess_mobile/src/widgets/settings.dart';
@@ -778,18 +779,26 @@ class _BottomBarState extends ConsumerState<_BottomBar> {
           makeLabel: (context) =>
               Text(context.l10n.puzzleFromGameLink(puzzleState.puzzle.game.id.value)),
           onPressed: () async {
-            final game = await ref.read(archivedGameProvider(puzzleState.puzzle.game.id).future);
-            if (context.mounted) {
-              Navigator.of(context).push(
-                AnalysisScreen.buildRoute(
-                  context,
-                  AnalysisOptions.archivedGame(
-                    orientation: puzzleState.pov,
-                    gameId: game.id,
-                    initialMoveCursor: puzzleState.puzzle.puzzle.initialPly + 1,
+            try {
+              final game = await ref
+                  .read(gameRepositoryProvider)
+                  .getGame(puzzleState.puzzle.game.id);
+              if (context.mounted) {
+                Navigator.of(context).push(
+                  AnalysisScreen.buildRoute(
+                    context,
+                    AnalysisOptions.archivedGame(
+                      orientation: puzzleState.pov,
+                      gameId: game.id,
+                      initialMoveCursor: puzzleState.puzzle.puzzle.initialPly + 1,
+                    ),
                   ),
-                ),
-              );
+                );
+              }
+            } catch (_) {
+              if (context.mounted) {
+                showSnackBar(context, 'Could not load the game', type: SnackBarType.error);
+              }
             }
           },
         ),

test/view/game/game_screen_test.dart

@@ -38,6 +38,10 @@ import '../../test_provider_scope.dart';
 final client = MockClient((request) {
   if (request.url.path == '/api/board/seek') {
     return mockResponse('ok', 200);
+  } else if (request.url.path == '/game/export/CCW6EEru') {
+    return mockResponse('''
+{"id":"CCW6EEru","rated":true,"source":"lobby","variant":"standard","speed":"bullet","perf":"bullet","createdAt":1706185945680,"lastMoveAt":1706186170504,"status":"resign","players":{"white":{"user":{"name":"veloce","id":"veloce"},"rating":1789,"ratingDiff":9},"black":{"user":{"name":"chabrot","id":"chabrot"},"rating":1810,"ratingDiff":-9}},"winner":"white","opening":{"eco":"C52","name":"Italian Game: Evans Gambit, Main Line","ply":10},"moves":"e4 e5 Nf3 Nc6 Bc4 Bc5 b4 Bxb4 c3 Ba5 d4 Bb6 Ba3 Nf6 Qb3 d6 Bxf7+ Kf8 O-O Qe7 Nxe5 Nxe5 dxe5 Be6 Bxe6 Nxe4 Re1 Nc5 Bxc5 Bxc5 Qxb7 Re8 Bh3 dxe5 Qf3+ Kg8 Nd2 Rf8 Qd5+ Rf7 Be6 Qxe6 Qxe6","clocks":[12003,12003,11883,11811,11683,11379,11307,11163,11043,11043,10899,10707,10155,10483,10019,9995,9635,9923,8963,8603,7915,8283,7763,7459,7379,6083,6587,5819,6363,5651,6075,5507,5675,4803,5059,4515,4547,3555,3971,3411,3235,3123,3120,2742],"clock":{"initial":120,"increment":1,"totalTime":160}}
+''', 200);
   }
   return mockResponse('', 404);
 });
@@ -808,6 +812,7 @@ void main() {
         find.widgetWithText(AppBar, 'Analysis board'),
         findsOneWidget,
       ); // analysis screen is now open
+      expect(find.byType(Chessboard), findsOneWidget);
       expect(find.byKey(const Key('e6-whitequeen')), findsOneWidget);
       expect(find.byKey(const Key('d5-lastMove')), findsOneWidget);
       expect(find.byKey(const Key('e6-lastMove')), findsOneWidget);