refactor(mobile): move error details to separate DB column (#6898)

* Add "details" column to LoggerMessage * Include error details in log details page * Move error details out of log message * Add error message to mixin * Create extension for HTTP Response logging * Fix analyze errors * format * fix analyze errors, format again
immich-app · Feb 24, 2024 · bc39790 · bc39790
1 parent 878932f
commit bc39790
Show file tree

Hide file tree

Showing 28 changed files with 319 additions and 161 deletions.
diff --git a/mobile/lib/extensions/asyncvalue_extensions.dart b/mobile/lib/extensions/asyncvalue_extensions.dart
@@ -30,7 +30,7 @@ extension LogOnError<T> on AsyncValue<T> {
     }
 
     if (hasError && !hasValue) {
-      _asyncErrorLogger.severe("$error", error, stackTrace);
+      _asyncErrorLogger.severe('Could not load value', error, stackTrace);
       return onError?.call(error, stackTrace) ??
           ScaffoldErrorBody(errorMsg: error?.toString());
     }

diff --git a/mobile/lib/extensions/response_extensions.dart b/mobile/lib/extensions/response_extensions.dart
@@ -0,0 +1,5 @@
+import 'package:http/http.dart';
+
+extension LoggerExtension on Response {
+  String toLoggerString() => "Status: $statusCode $reasonPhrase\n\n$body";
+}
diff --git a/mobile/lib/main.dart b/mobile/lib/main.dart
@@ -73,15 +73,14 @@ Future<void> initApp() async {
   FlutterError.onError = (details) {
     FlutterError.presentError(details);
     log.severe(
-      'FlutterError - Catch all error: ${details.toString()} - ${details.exception} - ${details.library} - ${details.context} - ${details.stack}',
-      details,
+      'FlutterError - Catch all',
+      "${details.toString()}\nException: ${details.exception}\nLibrary: ${details.library}\nContext: ${details.context}",
       details.stack,
     );
   };
 
   PlatformDispatcher.instance.onError = (error, stack) {
-    log.severe('PlatformDispatcher - Catch all error: $error', error, stack);
-    debugPrint("PlatformDispatcher - Catch all error: $error $stack");
+    log.severe('PlatformDispatcher - Catch all', error, stack);
     return true;
   };
 

diff --git a/mobile/lib/mixins/error_logger.mixin.dart b/mobile/lib/mixins/error_logger.mixin.dart
@@ -10,13 +10,14 @@ mixin ErrorLoggerMixin {
   /// Else, logs the error to the overrided logger and returns an AsyncError<>
   AsyncFuture<T> guardError<T>(
     Future<T> Function() fn, {
+    required String errorMessage,
     Level logLevel = Level.SEVERE,
   }) async {
     try {
       final result = await fn();
       return AsyncData(result);
     } catch (error, stackTrace) {
-      logger.log(logLevel, "$error", error, stackTrace);
+      logger.log(logLevel, errorMessage, error, stackTrace);
       return AsyncError(error, stackTrace);
     }
   }
@@ -26,12 +27,13 @@ mixin ErrorLoggerMixin {
   Future<T> logError<T>(
     Future<T> Function() fn, {
     required T defaultValue,
+    required String errorMessage,
     Level logLevel = Level.SEVERE,
   }) async {
     try {
       return await fn();
     } catch (error, stackTrace) {
-      logger.log(logLevel, "$error", error, stackTrace);
+      logger.log(logLevel, errorMessage, error, stackTrace);
     }
     return defaultValue;
   }

diff --git a/mobile/lib/modules/activities/services/activity.service.dart b/mobile/lib/modules/activities/services/activity.service.dart
@@ -24,6 +24,7 @@ class ActivityService with ErrorLoggerMixin {
         return list != null ? list.map(Activity.fromDto).toList() : [];
       },
       defaultValue: [],
+      errorMessage: "Failed to get all activities for album $albumId",
     );
   }
 
@@ -35,6 +36,7 @@ class ActivityService with ErrorLoggerMixin {
         return dto?.comments ?? 0;
       },
       defaultValue: 0,
+      errorMessage: "Failed to statistics for album $albumId",
     );
   }
 
@@ -45,6 +47,7 @@ class ActivityService with ErrorLoggerMixin {
         return true;
       },
       defaultValue: false,
+      errorMessage: "Failed to delete activity",
     );
   }
 
@@ -54,21 +57,24 @@ class ActivityService with ErrorLoggerMixin {
     String? assetId,
     String? comment,
   }) async {
-    return guardError(() async {
-      final dto = await _apiService.activityApi.createActivity(
-        ActivityCreateDto(
-          albumId: albumId,
-          type: type == ActivityType.comment
-              ? ReactionType.comment
-              : ReactionType.like,
-          assetId: assetId,
-          comment: comment,
-        ),
-      );
-      if (dto != null) {
-        return Activity.fromDto(dto);
-      }
-      throw NoResponseDtoError();
-    });
+    return guardError(
+      () async {
+        final dto = await _apiService.activityApi.createActivity(
+          ActivityCreateDto(
+            albumId: albumId,
+            type: type == ActivityType.comment
+                ? ReactionType.comment
+                : ReactionType.like,
+            assetId: assetId,
+            comment: comment,
+          ),
+        );
+        if (dto != null) {
+          return Activity.fromDto(dto);
+        }
+        throw NoResponseDtoError();
+      },
+      errorMessage: "Failed to create $type for album $albumId",
+    );
   }
 }
diff --git a/mobile/lib/modules/asset_viewer/services/image_viewer.service.dart b/mobile/lib/modules/asset_viewer/services/image_viewer.service.dart
@@ -1,6 +1,7 @@
 import 'dart:io';
 
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/response_extensions.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/providers/api.provider.dart';
 import 'package:immich_mobile/shared/services/api.service.dart';
@@ -39,7 +40,8 @@ class ImageViewerService {
           final failedResponse =
               imageResponse.statusCode != 200 ? imageResponse : motionReponse;
           _log.severe(
-            "Motion asset download failed with status - ${failedResponse.statusCode} and response - ${failedResponse.body}",
+            "Motion asset download failed",
+            failedResponse.toLoggerString(),
           );
           return false;
         }
@@ -75,9 +77,7 @@ class ImageViewerService {
             .downloadFileWithHttpInfo(asset.remoteId!);
 
         if (res.statusCode != 200) {
-          _log.severe(
-            "Asset download failed with status - ${res.statusCode} and response - ${res.body}",
-          );
+          _log.severe("Asset download failed", res.toLoggerString());
           return false;
         }
 
@@ -98,7 +98,7 @@ class ImageViewerService {
         return entity != null;
       }
     } catch (error, stack) {
-      _log.severe("Error saving file ${error.toString()}", error, stack);
+      _log.severe("Error saving downloaded asset", error, stack);
       return false;
     } finally {
       // Clear temp files

diff --git a/mobile/lib/modules/asset_viewer/ui/description_input.dart b/mobile/lib/modules/asset_viewer/ui/description_input.dart
@@ -48,7 +48,7 @@ class DescriptionInput extends HookConsumerWidget {
         );
       } catch (error, stack) {
         hasError.value = true;
-        _log.severe("Error updating description $error", error, stack);
+        _log.severe("Error updating description", error, stack);
         ImmichToast.show(
           context: context,
           msg: "description_input_submit_error".tr(),

diff --git a/mobile/lib/modules/backup/providers/backup.provider.dart b/mobile/lib/modules/backup/providers/backup.provider.dart
@@ -245,7 +245,7 @@ class BackupNotifier extends StateNotifier<BackUpState> {
         } catch (e, stack) {
           log.severe(
             "Failed to get thumbnail for album ${album.name}",
-            e.toString(),
+            e,
             stack,
           );
         }

diff --git a/mobile/lib/modules/login/providers/authentication.provider.dart b/mobile/lib/modules/login/providers/authentication.provider.dart
@@ -108,7 +108,7 @@ class AuthenticationNotifier extends StateNotifier<AuthenticationState> {
           .then((_) => log.info("Logout was successful for $userEmail"))
           .onError(
             (error, stackTrace) =>
-                log.severe("Error logging out $userEmail", error, stackTrace),
+                log.severe("Logout failed for $userEmail", error, stackTrace),
           );
 
       await Future.wait([
@@ -129,8 +129,8 @@ class AuthenticationNotifier extends StateNotifier<AuthenticationState> {
         shouldChangePassword: false,
         isAuthenticated: false,
       );
-    } catch (e) {
-      log.severe("Error logging out $e");
+    } catch (e, stack) {
+      log.severe('Logout failed', e, stack);
     }
   }
 

diff --git a/mobile/lib/modules/login/services/oauth.service.dart b/mobile/lib/modules/login/services/oauth.service.dart
@@ -36,7 +36,7 @@ class OAuthService {
         ),
       );
     } catch (e, stack) {
-      log.severe("Error performing oAuthLogin: ${e.toString()}", e, stack);
+      log.severe("OAuth login failed", e, stack);
       return null;
     }
   }

diff --git a/mobile/lib/modules/map/providers/map_state.provider.dart b/mobile/lib/modules/map/providers/map_state.provider.dart
@@ -1,6 +1,7 @@
 import 'dart:io';
 
 import 'package:flutter/material.dart';
+import 'package:immich_mobile/extensions/response_extensions.dart';
 import 'package:immich_mobile/modules/map/models/map_state.model.dart';
 import 'package:immich_mobile/modules/settings/providers/app_settings.provider.dart';
 import 'package:immich_mobile/modules/settings/services/app_settings.service.dart';
@@ -51,7 +52,8 @@ class MapStateNotifier extends _$MapStateNotifier {
         lightStyleFetched: AsyncError(lightResponse.body, StackTrace.current),
       );
       _log.severe(
-        "Cannot fetch map light style with status - ${lightResponse.statusCode} and response - ${lightResponse.body}",
+        "Cannot fetch map light style",
+        lightResponse.toLoggerString(),
       );
       return;
     }
@@ -77,9 +79,7 @@ class MapStateNotifier extends _$MapStateNotifier {
       state = state.copyWith(
         darkStyleFetched: AsyncError(darkResponse.body, StackTrace.current),
       );
-      _log.severe(
-        "Cannot fetch map dark style with status - ${darkResponse.statusCode} and response - ${darkResponse.body}",
-      );
+      _log.severe("Cannot fetch map dark style", darkResponse.toLoggerString());
       return;
     }
 

diff --git a/mobile/lib/modules/map/services/map.service.dart b/mobile/lib/modules/map/services/map.service.dart
@@ -28,6 +28,7 @@ class MapSerivce with ErrorLoggerMixin {
         return markers?.map(MapMarker.fromDto) ?? [];
       },
       defaultValue: [],
+      errorMessage: "Failed to get map markers",
     );
   }
 }
diff --git a/mobile/lib/modules/map/utils/map_utils.dart b/mobile/lib/modules/map/utils/map_utils.dart
@@ -105,10 +105,8 @@ class MapUtils {
         timeLimit: const Duration(seconds: 5),
       );
       return (currentUserLocation, null);
-    } catch (error) {
-      _log.severe(
-        "Cannot get user's current location due to ${error.toString()}",
-      );
+    } catch (error, stack) {
+      _log.severe("Cannot get user's current location", error, stack);
       return (null, LocationPermission.unableToDetermine);
     }
   }

diff --git a/mobile/lib/modules/map/widgets/map_asset_grid.dart b/mobile/lib/modules/map/widgets/map_asset_grid.dart
@@ -147,7 +147,7 @@ class MapAssetGrid extends HookConsumerWidget {
                         },
                         error: (error, stackTrace) {
                           log.warning(
-                            "Cannot get assets in the current map bounds $error",
+                            "Cannot get assets in the current map bounds",
                             error,
                             stackTrace,
                           );

diff --git a/mobile/lib/modules/memories/services/memory.service.dart b/mobile/lib/modules/memories/services/memory.service.dart
@@ -47,7 +47,7 @@ class MemoryService {
 
       return memories.isNotEmpty ? memories : null;
     } catch (error, stack) {
-      log.severe("Cannot get memories ${error.toString()}", error, stack);
+      log.severe("Cannot get memories", error, stack);
       return null;
     }
   }

diff --git a/mobile/lib/modules/partner/services/partner.service.dart b/mobile/lib/modules/partner/services/partner.service.dart
@@ -40,7 +40,7 @@ class PartnerService {
         return userDtos.map((u) => User.fromPartnerDto(u)).toList();
       }
     } catch (e) {
-      _log.warning("failed to get partners for direction $direction:\n$e");
+      _log.warning("Failed to get partners for direction $direction", e);
     }
     return null;
   }
@@ -51,7 +51,7 @@ class PartnerService {
       partner.isPartnerSharedBy = false;
       await _db.writeTxn(() => _db.users.put(partner));
     } catch (e) {
-      _log.warning("failed to remove partner ${partner.id}:\n$e");
+      _log.warning("Failed to remove partner ${partner.id}", e);
       return false;
     }
     return true;
@@ -66,7 +66,7 @@ class PartnerService {
         return true;
       }
     } catch (e) {
-      _log.warning("failed to add partner ${partner.id}:\n$e");
+      _log.warning("Failed to add partner ${partner.id}", e);
     }
     return false;
   }
@@ -81,7 +81,7 @@ class PartnerService {
         return true;
       }
     } catch (e) {
-      _log.warning("failed to update partner ${partner.id}:\n$e");
+      _log.warning("Failed to update partner ${partner.id}", e);
     }
     return false;
   }

diff --git a/mobile/lib/modules/shared_link/services/shared_link.service.dart b/mobile/lib/modules/shared_link/services/shared_link.service.dart
@@ -22,7 +22,7 @@ class SharedLinkService {
           ? AsyncData(list.map(SharedLink.fromDto).toList())
           : const AsyncData([]);
     } catch (e, stack) {
-      _log.severe("failed to fetch shared links - $e");
+      _log.severe("Failed to fetch shared links", e, stack);
       return AsyncError(e, stack);
     }
   }
@@ -31,7 +31,7 @@ class SharedLinkService {
     try {
       return await _apiService.sharedLinkApi.removeSharedLink(id);
     } catch (e) {
-      _log.severe("failed to delete shared link id - $id with error - $e");
+      _log.severe("Failed to delete shared link id - $id", e);
     }
   }
 
@@ -81,7 +81,7 @@ class SharedLinkService {
         }
       }
     } catch (e) {
-      _log.severe("failed to create shared link with error - $e");
+      _log.severe("Failed to create shared link", e);
     }
     return null;
   }
@@ -113,7 +113,7 @@ class SharedLinkService {
         return SharedLink.fromDto(responseDto);
       }
     } catch (e) {
-      _log.severe("failed to update shared link id - $id with error - $e");
+      _log.severe("Failed to update shared link id - $id", e);
     }
     return null;
   }

diff --git a/mobile/lib/modules/trash/providers/trashed_asset.provider.dart b/mobile/lib/modules/trash/providers/trashed_asset.provider.dart
@@ -44,7 +44,7 @@ class TrashNotifier extends StateNotifier<bool> {
           .read(syncServiceProvider)
           .handleRemoteAssetRemoval(idsToRemove.cast<String>().toList());
     } catch (error, stack) {
-      _log.severe("Cannot empty trash ${error.toString()}", error, stack);
+      _log.severe("Cannot empty trash", error, stack);
     }
   }
 
@@ -70,7 +70,7 @@ class TrashNotifier extends StateNotifier<bool> {
 
       return isRemoved;
     } catch (error, stack) {
-      _log.severe("Cannot empty trash ${error.toString()}", error, stack);
+      _log.severe("Cannot remove assets", error, stack);
     }
     return false;
   }
@@ -93,7 +93,7 @@ class TrashNotifier extends StateNotifier<bool> {
         return true;
       }
     } catch (error, stack) {
-      _log.severe("Cannot restore trash ${error.toString()}", error, stack);
+      _log.severe("Cannot restore assets", error, stack);
     }
     return false;
   }
@@ -123,7 +123,7 @@ class TrashNotifier extends StateNotifier<bool> {
         await _db.assets.putAll(updatedAssets);
       });
     } catch (error, stack) {
-      _log.severe("Cannot restore trash ${error.toString()}", error, stack);
+      _log.severe("Cannot restore trash", error, stack);
     }
   }
 }