diff --git a/lib/database/tvos_database_recovery_store.dart b/lib/database/tvos_database_recovery_store.dart index 7e9affbf..73ede9a5 100644 --- a/lib/database/tvos_database_recovery_store.dart +++ b/lib/database/tvos_database_recovery_store.dart @@ -5,6 +5,7 @@ import 'package:flutter/foundation.dart'; import 'package:shared_preferences/shared_preferences.dart'; import '../utils/app_logger.dart'; +import '../services/base_shared_preferences_service.dart'; /// Startup result after reconciling the purgeable tvOS database with its /// bounded standard-domain recovery image. @@ -98,7 +99,14 @@ final class TvosDatabaseRecoveryStore { }) async { if (!isTvos) return TvosDatabaseRecoveryOutcome.notApplicable; - final recoveryRequired = _preferences.getBool(recoveryRequiredKey) ?? false; + // Tolerant read: `reconcile` runs inside `AppDatabase.open`, a fatal + // startup step, so a wrong-typed marker would veto the launch outright on + // a first-class TV target. An unreadable marker tells us nothing, which is + // the same position as an absent one — default false and drop the key + // (#1732). The manifest reads below are already inside catch-alls. + final recoveryRequired = + readPreferenceTolerantly(_preferences, recoveryRequiredKey, () => _preferences.getBool(recoveryRequiredKey)) ?? + false; if (recoveryRequired) { await _reloadManifestCacheIfNeeded(); final snapshot = _readCommittedSnapshot(); diff --git a/lib/main.dart b/lib/main.dart index 711a3dec..036c3e96 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -244,6 +244,10 @@ Future<_StartupDependencies> _initializeApplication() async { options.beforeBreadcrumb = _beforeBreadcrumb; }); }); + // Settings are loaded and the reporter is up, so the crash-reporting + // opt-out in `_beforeSend` now applies. This is the first moment a failure + // from an earlier attempt can actually be sent. + unawaited(flushPendingStartupFailure()); } AndroidExitDiagnostics.markTelemetryReady(); @@ -310,24 +314,51 @@ StartupFailureRecord describeStartupFailure(Object error, StackTrace stackTrace) repairable: _isRepairable(error), ); -/// Default [StartupBootstrap.reportFailure]. +/// Sends a persisted startup failure to the crash reporter, once. +/// +/// Reporting cannot happen where the failure is caught. The gate opens +/// preferences — the likeliest thing to fail, and the whole reason #1732 has +/// no telemetry — before crash reporting exists, so a `captureException` +/// there goes to a no-op hub and is silently dropped. Initialising the +/// reporter first is not an option either: `_beforeSend` reads the +/// crash-reporting opt-out from settings, which are not loaded yet, so early +/// events would bypass a user's choice. +/// +/// So every failure is persisted first and flushed here, immediately after +/// the reporter comes up with settings loaded. In practice that is the user's +/// own retry, seconds later, in the same process. @visibleForTesting -Future reportStartupFailure(StartupFailureRecord record, Object error, StackTrace stackTrace) async { - if (!_enableSentry) return; +Future flushPendingStartupFailure({Future Function(StartupFailureRecord record)? send}) async { + final record = await StartupDiagnosticsStore.peekPersisted(); + if (record == null || record.reported) return; try { - await Sentry.captureException( - error, - stackTrace: stackTrace, - withScope: (scope) { - scope.setTag('startup.phase', record.phaseId); - scope.level = SentryLevel.fatal; - }, - ); - } catch (reportError, reportStack) { - appLogger.d('Could not report the startup failure', error: reportError, stackTrace: reportStack); + await (send ?? _captureStartupFailure)(record); + await StartupDiagnosticsStore.markReported(record); + } catch (error, stackTrace) { + // Leave it unreported so the next launch tries again. + appLogger.d('Could not report the startup failure', error: error, stackTrace: stackTrace); } } +Future _captureStartupFailure(StartupFailureRecord record) async { + // The original error object is long gone by now; the record is the + // allowlisted, already-redacted rendering of it. + await Sentry.captureMessage( + 'Startup failed: ${record.headline}', + level: SentryLevel.fatal, + withScope: (scope) { + scope.setTag('startup.phase', record.phaseId); + scope.setContexts('startup', { + 'phase': record.phaseId, + 'errorType': record.errorType, + 'repairable': record.repairable, + 'when': record.timestamp.toUtc().toIso8601String(), + 'stackTrace': ?record.stackTrace, + }); + }, + ); +} + /// Default [StartupBootstrap.repair]: states the cost, runs the repair that /// matches the failure, then reports what was kept and what was lost. /// @@ -440,7 +471,6 @@ class StartupBootstrap extends StatefulWidget { this.discard, this.onCommitted, this.describeFailure = describeStartupFailure, - this.reportFailure = reportStartupFailure, this.repair = repairStartupStorage, this.lightTheme, this.darkTheme, @@ -456,10 +486,6 @@ class StartupBootstrap extends StatefulWidget { /// the persisted diagnostic and the crash report all share. final StartupFailureRecord Function(Object error, StackTrace stackTrace) describeFailure; - /// Sends the failure to the crash reporter. The gate catches the error, so - /// nothing else will. - final Future Function(StartupFailureRecord record, Object error, StackTrace stackTrace) reportFailure; - /// Offers the user a consented repair for a recoverable failure and reports /// whether one ran. Returning true re-runs [initialize]. final Future Function(BuildContext context, StartupFailureRecord record, Object error)? repair; @@ -530,10 +556,10 @@ class _StartupBootstrapState extends State> { // process. On Windows there is no log file and no console for a // double-clicked release build. unawaited(StartupDiagnosticsStore.record(failure)); - // The gate catches this error, so it is a *handled* Dart error that - // never reaches PlatformDispatcher.onError. Report it explicitly or the - // crash dashboard stays empty for the one failure that hides the app. - unawaited(widget.reportFailure(failure, error, stackTrace)); + // Not reported from here: the earliest phases run before the crash + // reporter exists, so the record is persisted and flushed by + // `flushPendingStartupFailure` once it is up — on the retry below, or on + // the next launch. if (!mounted || generation != _generation) return; setState(() { _failure = failure; diff --git a/lib/services/startup_diagnostics.dart b/lib/services/startup_diagnostics.dart index 4e56ddf8..a83b81fc 100644 --- a/lib/services/startup_diagnostics.dart +++ b/lib/services/startup_diagnostics.dart @@ -80,6 +80,7 @@ class StartupFailureRecord { required this.appVersion, required this.platform, this.repairable = false, + this.reported = false, }) : message = LogRedactionManager.redact(message), stackTrace = stackTrace == null ? null : LogRedactionManager.redact(stackTrace); @@ -144,6 +145,26 @@ class StartupFailureRecord { /// Whether the gate can offer an in-app repair for this failure. final bool repairable; + /// Whether this record has already reached the crash reporter. + /// + /// The earliest gate phases run before crash reporting is initialised, so a + /// failure there is captured by a no-op hub and silently discarded. Records + /// are therefore always persisted first and sent once the reporter is up — + /// on the in-app retry, or on the next launch (#1732). + final bool reported; + + StartupFailureRecord copyWith({bool? reported}) => StartupFailureRecord( + phase: phase, + errorType: errorType, + message: message, + stackTrace: stackTrace, + timestamp: timestamp, + appVersion: appVersion, + platform: platform, + repairable: repairable, + reported: reported ?? this.reported, + ); + String get phaseId => phase?.id ?? 'unknown'; /// One-line summary for the failure screen and the log. @@ -177,6 +198,7 @@ class StartupFailureRecord { 'appVersion': appVersion, 'platform': platform, 'repairable': repairable, + 'reported': reported, }; static StartupFailureRecord? fromJson(Map json) { @@ -193,6 +215,7 @@ class StartupFailureRecord { appVersion: json['appVersion'] as String? ?? 'unknown', platform: json['platform'] as String? ?? 'unknown', repairable: json['repairable'] as bool? ?? false, + reported: json['reported'] as bool? ?? false, ); } } @@ -243,6 +266,37 @@ abstract final class StartupDiagnosticsStore { } } + /// Reads a persisted record without consuming it. + /// + /// Used by the crash-report flush, which has to run before the record is + /// consumed for display and must not remove it if the send fails. + static Future peekPersisted() async { + try { + final file = await _file(); + if (file == null || !await file.exists()) return null; + final decoded = jsonDecode(await file.readAsString()); + if (decoded is! Map) return null; + return StartupFailureRecord.fromJson(decoded.cast()); + } catch (error, stackTrace) { + appLogger.d('Could not peek at the startup failure record', error: error, stackTrace: stackTrace); + return null; + } + } + + /// Rewrites the persisted record as already reported, so a later launch does + /// not send it a second time. It stays on disk for Settings > Logs. + static Future markReported(StartupFailureRecord failure) async { + final updated = failure.copyWith(reported: true); + if (_pending != null) _pending = updated; + try { + final file = await _file(); + if (file == null || !await file.exists()) return; + await file.writeAsString(jsonEncode(updated.toJson()), flush: true); + } catch (error, stackTrace) { + appLogger.d('Could not mark the startup failure record as reported', error: error, stackTrace: stackTrace); + } + } + /// Reads and deletes a record written by an earlier launch. /// /// Deleting on read stops one stale failure from following the user forever; diff --git a/test/services/startup_diagnostics_test.dart b/test/services/startup_diagnostics_test.dart index 434260cc..15383c64 100644 --- a/test/services/startup_diagnostics_test.dart +++ b/test/services/startup_diagnostics_test.dart @@ -2,6 +2,7 @@ import 'dart:convert'; import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/main.dart'; import 'package:plezy/services/sensitive_prefs.dart'; import 'package:plezy/services/startup_diagnostics.dart'; import 'package:plezy/utils/log_redaction_manager.dart'; @@ -189,4 +190,49 @@ void main() { expect(StartupPhase.fromId(null), isNull); }); }); + + group('deferred crash reporting', () { + // The preferences phase runs before crash reporting is initialised, so an + // inline capture goes to a no-op hub and is silently discarded — the + // likeliest failure phase producing no telemetry at all (#1732). + + test('flushes a persisted record once and marks it reported', () async { + await StartupDiagnosticsStore.record(_record(phase: StartupPhase.preferences)); + + final sent = []; + await flushPendingStartupFailure(send: (record) async => sent.add(record)); + + expect(sent.single.phase, StartupPhase.preferences); + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isTrue); + + // A later launch must not resend it. + await flushPendingStartupFailure(send: (record) async => sent.add(record)); + expect(sent, hasLength(1)); + }); + + test('leaves the record unreported when the send fails', () async { + await StartupDiagnosticsStore.record(_record()); + + await flushPendingStartupFailure(send: (_) async => throw StateError('offline')); + + // Still pending, so the next launch retries rather than losing it. + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isFalse); + }); + + test('does nothing when no launch has failed', () async { + final sent = []; + + await flushPendingStartupFailure(send: (record) async => sent.add(record)); + + expect(sent, isEmpty); + }); + + test('peeking leaves the record on disk for the display path', () async { + await StartupDiagnosticsStore.record(_record()); + + await StartupDiagnosticsStore.peekPersisted(); + + expect(await StartupDiagnosticsStore.consumePrevious(), isNotNull); + }); + }); } diff --git a/test/startup_bootstrap_test.dart b/test/startup_bootstrap_test.dart index b29f0393..7100ffa2 100644 --- a/test/startup_bootstrap_test.dart +++ b/test/startup_bootstrap_test.dart @@ -97,7 +97,6 @@ void main() { StartupBootstrap( initialize: () async => throw const StartupPhaseException(StartupPhase.database, FormatException('boom')), buildApp: (_, value) => Text('ready $value'), - reportFailure: (_, _, _) async {}, ), ); await tester.pump(); @@ -115,7 +114,6 @@ void main() { StartupBootstrap( initialize: () async => throw StateError('database unavailable'), buildApp: (_, value) => Text('ready $value'), - reportFailure: (_, _, _) async {}, ), ); await tester.pump(); @@ -133,7 +131,6 @@ void main() { StartupBootstrap( initialize: () async => throw StateError('unrelated'), buildApp: (_, value) => Text('ready $value'), - reportFailure: (_, _, _) async {}, ), ); await tester.pump(); @@ -144,7 +141,6 @@ void main() { key: const Key('repairable'), initialize: () async => throw CorruptPreferenceStoreException(const FormatException('bad'), StackTrace.current), buildApp: (_, value) => Text('ready $value'), - reportFailure: (_, _, _) async {}, ), ); await tester.pump(); @@ -165,7 +161,6 @@ void main() { return 7; }, buildApp: (_, value) => MaterialApp(home: Text('ready $value')), - reportFailure: (_, _, _) async {}, repair: (_, _, _) async { repairCalls++; return true; @@ -193,7 +188,6 @@ void main() { throw CorruptPreferenceStoreException(const FormatException('bad'), StackTrace.current); }, buildApp: (_, value) => MaterialApp(home: Text('ready $value')), - reportFailure: (_, _, _) async {}, repair: (_, _, _) async => false, ), ); @@ -207,20 +201,23 @@ void main() { expect(find.byKey(startupBootstrapFailureKey), findsOneWidget); }); - testWidgets('reports the failure to the crash reporter', (tester) async { - StartupFailureRecord? reported; + testWidgets('persists the failure so it can be reported once the reporter is up', (tester) async { + // Not reported inline: the earliest gate phases run before crash + // reporting exists, so an inline capture would reach a no-op hub and be + // discarded. The record is held for `flushPendingStartupFailure` (#1732). + StartupDiagnosticsStore.resetForTesting(); + addTearDown(StartupDiagnosticsStore.resetForTesting); await tester.pumpWidget( StartupBootstrap( initialize: () async => throw const StartupPhaseException(StartupPhase.storage, 'nope'), buildApp: (_, value) => Text('ready $value'), - reportFailure: (record, _, _) async => reported = record, ), ); await tester.pump(); - // The gate catches the error, so nothing else would ever see it. - expect(reported?.phase, StartupPhase.storage); + expect(StartupDiagnosticsStore.pending?.phase, StartupPhase.storage); + expect(StartupDiagnosticsStore.pending?.reported, isFalse); }); testWidgets('retry clears the failed generation and can commit a later success', (tester) async {