From ffbf0bd8f278f8fe307c8dda222b400da6db120d Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Sat, 23 Nov 2024 23:24:41 +0100 Subject: [PATCH] improved error reporting --- lib/model/entry/extensions/metadata_edition.dart | 4 ++-- lib/model/media/video/metadata.dart | 4 ++-- lib/model/source/media_store_source.dart | 6 +++--- lib/model/source/trash.dart | 2 +- lib/model/vaults/vaults.dart | 2 +- lib/services/metadata/metadata_edit_service.dart | 10 +++------- lib/widgets/aves_app.dart | 2 +- lib/widgets/viewer/action/single_entry_editor.dart | 4 ++-- lib/widgets/viewer/controls/cast.dart | 4 ++-- plugins/aves_report/lib/aves_report.dart | 2 +- .../aves_report_console/lib/aves_report_platform.dart | 2 +- .../lib/aves_report_platform.dart | 4 +++- plugins/aves_report_crashlytics/pubspec.lock | 2 +- plugins/aves_report_crashlytics/pubspec.yaml | 1 + test/fake/report_service.dart | 2 +- 15 files changed, 25 insertions(+), 26 deletions(-) diff --git a/lib/model/entry/extensions/metadata_edition.dart b/lib/model/entry/extensions/metadata_edition.dart index 6ad3b2dce..5e4228279 100644 --- a/lib/model/entry/extensions/metadata_edition.dart +++ b/lib/model/entry/extensions/metadata_edition.dart @@ -32,7 +32,7 @@ extension ExtraAvesEntryMetadataEdition on AvesEntry { final appliedModifier = await _applyDateModifierToEntry(userModifier); if (appliedModifier == null) { if (isValid && userModifier.action != DateEditAction.copyField) { - await reportService.recordError('failed to get date for modifier=$userModifier, entry=$this', null); + await reportService.recordError('failed to get date for modifier=$userModifier, entry=$this'); } return {}; } @@ -65,7 +65,7 @@ extension ExtraAvesEntryMetadataEdition on AvesEntry { final shiftedDate = date.add(Duration(seconds: appliedModifier.shiftSeconds!)); editCreateDateXmp(descriptions, shiftedDate); } else { - reportService.recordError('failed to parse XMP date=$xmpDate', null); + reportService.recordError('failed to parse XMP date=$xmpDate'); } } case DateEditAction.remove: diff --git a/lib/model/media/video/metadata.dart b/lib/model/media/video/metadata.dart index 3349e202f..972f227ba 100644 --- a/lib/model/media/video/metadata.dart +++ b/lib/model/media/video/metadata.dart @@ -1,12 +1,12 @@ import 'dart:async'; import 'package:aves/model/entry/entry.dart'; -import 'package:aves/model/metadata/catalog.dart'; import 'package:aves/model/media/video/channel_layouts.dart'; import 'package:aves/model/media/video/codecs.dart'; import 'package:aves/model/media/video/profiles/aac.dart'; import 'package:aves/model/media/video/profiles/h264.dart'; import 'package:aves/model/media/video/profiles/hevc.dart'; +import 'package:aves/model/metadata/catalog.dart'; import 'package:aves/ref/languages.dart'; import 'package:aves/ref/locales.dart'; import 'package:aves/ref/mime_types.dart'; @@ -99,7 +99,7 @@ class VideoMetadataFormatter { if (isDefined(dateString)) { dateMillis = parseVideoDate(dateString); if (dateMillis == null && !isAmbiguousDate(dateString)) { - await reportService.recordError('getCatalogMetadata failed to parse date=$dateString for mimeType=${entry.mimeType} entry=$entry', null); + await reportService.recordError('getCatalogMetadata failed to parse date=$dateString for mimeType=${entry.mimeType} entry=$entry'); } } diff --git a/lib/model/source/media_store_source.dart b/lib/model/source/media_store_source.dart index f9affcfb7..9f01ef478 100644 --- a/lib/model/source/media_store_source.dart +++ b/lib/model/source/media_store_source.dart @@ -62,7 +62,7 @@ class MediaStoreSource extends CollectionSource { final currentTimeZoneOffset = DateTime.now().timeZoneOffset.inMilliseconds; final catalogTimeZoneOffset = settings.catalogTimeZoneOffsetMillis; if (currentTimeZoneOffset != catalogTimeZoneOffset) { - unawaited(reportService.recordError('Time zone offset change: $currentTimeZoneOffset -> $catalogTimeZoneOffset. Clear catalog metadata to get correct date/times.', null)); + unawaited(reportService.recordError('Time zone offset change: $currentTimeZoneOffset -> $catalogTimeZoneOffset. Clear catalog metadata to get correct date/times.')); await localMediaDb.clearDates(); await localMediaDb.clearCatalogMetadata(); settings.catalogTimeZoneOffsetMillis = currentTimeZoneOffset; @@ -212,7 +212,7 @@ class MediaStoreSource extends CollectionSource { // TODO TLAD find duplication cause final duplicates = await localMediaDb.searchLiveDuplicates(EntryOrigins.mediaStoreContent, newEntries); if (duplicates.isNotEmpty) { - unawaited(reportService.recordError(Exception('Loading entries yielded duplicates=${duplicates.join(', ')}'), StackTrace.current)); + unawaited(reportService.recordError(Exception('Loading entries yielded duplicates=${duplicates.join(', ')}'))); // post-error cleanup await localMediaDb.removeIds(duplicates.map((v) => v.id).toSet()); for (final duplicate in duplicates) { @@ -325,7 +325,7 @@ class MediaStoreSource extends CollectionSource { // TODO TLAD find duplication cause final duplicates = await localMediaDb.searchLiveDuplicates(EntryOrigins.mediaStoreContent, newEntries); if (duplicates.isNotEmpty) { - unawaited(reportService.recordError(Exception('Refreshing entries yielded duplicates=${duplicates.join(', ')}'), StackTrace.current)); + unawaited(reportService.recordError(Exception('Refreshing entries yielded duplicates=${duplicates.join(', ')}'))); // post-error cleanup await localMediaDb.removeIds(duplicates.map((v) => v.id).toSet()); for (final duplicate in duplicates) { diff --git a/lib/model/source/trash.dart b/lib/model/source/trash.dart index f3107cbf5..6afbce6be 100644 --- a/lib/model/source/trash.dart +++ b/lib/model/source/trash.dart @@ -76,7 +76,7 @@ mixin TrashMixin on SourceBase { sourceEntry.trashDetails = _buildTrashDetails(id); newEntries.add(sourceEntry); } else { - await reportService.recordError('Failed to recover untracked bin item at uri=$uri', null); + await reportService.recordError('Failed to recover untracked bin item at uri=$uri'); } } }); diff --git a/lib/model/vaults/vaults.dart b/lib/model/vaults/vaults.dart index a621c2952..7f80c563a 100644 --- a/lib/model/vaults/vaults.dart +++ b/lib/model/vaults/vaults.dart @@ -175,7 +175,7 @@ class Vaults extends ChangeNotifier { sourceEntry.origin = EntryOrigins.vault; newEntries.add(sourceEntry); } else { - await reportService.recordError('Failed to recover untracked vault item at uri=$uri', null); + await reportService.recordError('Failed to recover untracked vault item at uri=$uri'); } }); } diff --git a/lib/services/metadata/metadata_edit_service.dart b/lib/services/metadata/metadata_edit_service.dart index 7317e55e8..611b4cbc1 100644 --- a/lib/services/metadata/metadata_edit_service.dart +++ b/lib/services/metadata/metadata_edit_service.dart @@ -6,10 +6,8 @@ import 'package:aves/model/entry/extensions/props.dart'; import 'package:aves/model/metadata/date_modifier.dart'; import 'package:aves/services/common/services.dart'; import 'package:aves_model/aves_model.dart'; -import 'package:aves_report/aves_report.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; -import 'package:stack_trace/stack_trace.dart'; abstract class MetadataEditService { Future> rotate(AvesEntry entry, {required bool clockwise}); @@ -135,24 +133,22 @@ class PlatformMetadataEditService implements MetadataEditService { } } - StackTrace? _currentStack() => ReportService.buildReportStack(Trace.current(), level: 1); - // distinct exceptions to convince Crashlytics to split reports into distinct issues // The distinct debug statement is there to make the body unique, so that the methods are not merged at compile time. Future mp4LargeMoov(CustomPlatformException e) { debugPrint('mp4LargeMoov $e'); - return reportService.recordError(e, _currentStack()); + return reportService.recordError(e); } Future mp4LargeOther(CustomPlatformException e) { debugPrint('mp4LargeOther $e'); - return reportService.recordError(e, _currentStack()); + return reportService.recordError(e); } Future fileNotFound(CustomPlatformException e) { debugPrint('fileNotFound $e'); - return reportService.recordError(e, _currentStack()); + return reportService.recordError(e); } } diff --git a/lib/widgets/aves_app.dart b/lib/widgets/aves_app.dart index cc1179ffd..7eedcdc33 100644 --- a/lib/widgets/aves_app.dart +++ b/lib/widgets/aves_app.dart @@ -693,7 +693,7 @@ class _AvesAppState extends State with WidgetsBindingObserver { _mediaStoreSource.updateDerivedFilters(); } - void _onError(String? error) => reportService.recordError(error, null); + void _onError(String? error) => reportService.recordError(error); void _onAppModeChanged() { final appMode = _appModeNotifier.value; diff --git a/lib/widgets/viewer/action/single_entry_editor.dart b/lib/widgets/viewer/action/single_entry_editor.dart index 85acb841f..46adba957 100644 --- a/lib/widgets/viewer/action/single_entry_editor.dart +++ b/lib/widgets/viewer/action/single_entry_editor.dart @@ -61,8 +61,8 @@ mixin SingleEntryEditorMixin on FeedbackMixin, PermissionAwareMixin { } else { showFeedback(context, FeedbackType.warn, l10n.genericFailureFeedback); } - } catch (error, stack) { - await reportService.recordError(error, stack); + } catch (e, stack) { + await reportService.recordError(e, stack); } source?.resumeMonitoring(); } diff --git a/lib/widgets/viewer/controls/cast.dart b/lib/widgets/viewer/controls/cast.dart index 731712a59..377f8714d 100644 --- a/lib/widgets/viewer/controls/cast.dart +++ b/lib/widgets/viewer/controls/cast.dart @@ -96,8 +96,8 @@ mixin CastMixin { ); debugPrint('cast: play entry=$entry'); unawaited(renderer.play()); - } catch (error, stack) { - await reportService.recordError(error, stack); + } catch (e, stack) { + await reportService.recordError(e, stack); } } diff --git a/plugins/aves_report/lib/aves_report.dart b/plugins/aves_report/lib/aves_report.dart index 4d3a3fc5c..609252260 100644 --- a/plugins/aves_report/lib/aves_report.dart +++ b/plugins/aves_report/lib/aves_report.dart @@ -15,7 +15,7 @@ abstract class ReportService { Future setCustomKeys(Map map); - Future recordError(dynamic exception, StackTrace? stack); + Future recordError(dynamic exception, [StackTrace? stack]); Future recordFlutterError(FlutterErrorDetails flutterErrorDetails); diff --git a/plugins/aves_report_console/lib/aves_report_platform.dart b/plugins/aves_report_console/lib/aves_report_platform.dart index e57e70f85..dd1d461ea 100644 --- a/plugins/aves_report_console/lib/aves_report_platform.dart +++ b/plugins/aves_report_console/lib/aves_report_platform.dart @@ -9,7 +9,7 @@ class PlatformReportService extends ReportService { Future log(String message) async => debugPrint('Report log with message=$message'); @override - Future recordError(dynamic exception, StackTrace? stack) async => debugPrint('Report error with exception=$exception, stack=$stack'); + Future recordError(dynamic exception, [StackTrace? stack]) async => debugPrint('Report error with exception=$exception, stack=$stack'); @override Future recordFlutterError(FlutterErrorDetails flutterErrorDetails) async => debugPrint('Report Flutter error with details=$flutterErrorDetails'); diff --git a/plugins/aves_report_crashlytics/lib/aves_report_platform.dart b/plugins/aves_report_crashlytics/lib/aves_report_platform.dart index 1bcd7f567..05a7efbf6 100644 --- a/plugins/aves_report_crashlytics/lib/aves_report_platform.dart +++ b/plugins/aves_report_crashlytics/lib/aves_report_platform.dart @@ -5,6 +5,7 @@ import 'package:firebase_core/firebase_core.dart'; import 'package:firebase_crashlytics/firebase_crashlytics.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; +import 'package:stack_trace/stack_trace.dart'; class PlatformReportService extends ReportService { FirebaseCrashlytics? get _instance { @@ -65,11 +66,12 @@ class PlatformReportService extends ReportService { } @override - Future recordError(dynamic exception, StackTrace? stack) async { + Future recordError(dynamic exception, [StackTrace? stack]) async { if (exception is PlatformException && stack != null) { stack = ReportService.buildReportStack(stack, level: 2); } if (exception is! UnreportedStateError) { + stack ??= ReportService.buildReportStack(Trace.current(), level: 1); return _instance?.recordError(exception, stack); } } diff --git a/plugins/aves_report_crashlytics/pubspec.lock b/plugins/aves_report_crashlytics/pubspec.lock index da24c1b41..91fcf4ed0 100644 --- a/plugins/aves_report_crashlytics/pubspec.lock +++ b/plugins/aves_report_crashlytics/pubspec.lock @@ -213,7 +213,7 @@ packages: source: hosted version: "1.10.0" stack_trace: - dependency: transitive + dependency: "direct main" description: name: stack_trace sha256: "73713990125a6d93122541237550ee3352a2d84baad52d375a4cad2eb9b7ce0b" diff --git a/plugins/aves_report_crashlytics/pubspec.yaml b/plugins/aves_report_crashlytics/pubspec.yaml index c94c91d59..5c04ca756 100644 --- a/plugins/aves_report_crashlytics/pubspec.yaml +++ b/plugins/aves_report_crashlytics/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: # so that the transitive `path` gets upgraded to v1.8.3 firebase_core: ">=2.10.0" firebase_crashlytics: + stack_trace: dev_dependencies: flutter_lints: diff --git a/test/fake/report_service.dart b/test/fake/report_service.dart index 898c9462c..a7a8f99b2 100644 --- a/test/fake/report_service.dart +++ b/test/fake/report_service.dart @@ -21,7 +21,7 @@ class FakeReportService extends ReportService { Future setCustomKeys(Map map) => SynchronousFuture(null); @override - Future recordError(dynamic exception, StackTrace? stack) => SynchronousFuture(null); + Future recordError(dynamic exception, [StackTrace? stack]) => SynchronousFuture(null); @override Future recordFlutterError(FlutterErrorDetails flutterErrorDetails) => SynchronousFuture(null);