Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion mobile/lib/extensions/asyncvalue_extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
5 changes: 5 additions & 0 deletions mobile/lib/extensions/response_extensions.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import 'package:http/http.dart';

extension LoggerExtension on Response {
String toLoggerString() => "Status: $statusCode $reasonPhrase\n\n$body";
}
7 changes: 3 additions & 4 deletions mobile/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
6 changes: 4 additions & 2 deletions mobile/lib/mixins/error_logger.mixin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -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;
}
Expand Down
38 changes: 22 additions & 16 deletions mobile/lib/modules/activities/services/activity.service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
}

Expand All @@ -35,6 +36,7 @@ class ActivityService with ErrorLoggerMixin {
return dto?.comments ?? 0;
},
defaultValue: 0,
errorMessage: "Failed to statistics for album $albumId",
);
}

Expand All @@ -45,6 +47,7 @@ class ActivityService with ErrorLoggerMixin {
return true;
},
defaultValue: false,
errorMessage: "Failed to delete activity",
);
}

Expand All @@ -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",
);
}
}
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion mobile/lib/modules/asset_viewer/ui/description_input.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion mobile/lib/modules/backup/providers/backup.provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion mobile/lib/modules/login/services/oauth.service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
8 changes: 4 additions & 4 deletions mobile/lib/modules/map/providers/map_state.provider.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions mobile/lib/modules/map/services/map.service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class MapSerivce with ErrorLoggerMixin {
return markers?.map(MapMarker.fromDto) ?? [];
},
defaultValue: [],
errorMessage: "Failed to get map markers",
);
}
}
6 changes: 2 additions & 4 deletions mobile/lib/modules/map/utils/map_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion mobile/lib/modules/map/widgets/map_asset_grid.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down
2 changes: 1 addition & 1 deletion mobile/lib/modules/memories/services/memory.service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
8 changes: 4 additions & 4 deletions mobile/lib/modules/partner/services/partner.service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
}
Expand Down