diff --git a/lib/main.dart b/lib/main.dart index 036c3e96..4fe89f31 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -243,13 +243,19 @@ Future<_StartupDependencies> _initializeApplication() async { options.beforeSend = _beforeSend; options.beforeBreadcrumb = _beforeBreadcrumb; }); + // Only reached when init really completed; the phase above swallows a + // failure or a timeout and would otherwise leave a no-op hub that + // accepts events and drops them. + _crashReporterReady = true; }); - // 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()); } + // Registered rather than awaited: sending must not sit on the critical path, + // but it must finish before the success path consumes the record off disk. + // Runs even without Sentry so a build that can never report still resolves + // the record instead of carrying it forever. + StartupDiagnosticsStore.holdForFlush(flushPendingStartupFailure()); + AndroidExitDiagnostics.markTelemetryReady(); return _initializeStartup(settings); } @@ -314,25 +320,75 @@ StartupFailureRecord describeStartupFailure(Object error, StackTrace stackTrace) repairable: _isRepairable(error), ); +/// Whether `SentryFlutter.init` actually completed this launch. +/// +/// The crash-reporting phase is best-effort, so init can fail or time out and +/// leave a no-op hub behind. A no-op hub accepts `captureMessage` and returns +/// an empty id *without throwing*, so "the send did not throw" is not evidence +/// that anything was sent. +var _crashReporterReady = false; + +@visibleForTesting +void debugSetCrashReporterReady(bool ready) => _crashReporterReady = ready; + /// 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. +/// no telemetry — before crash reporting exists, so a capture 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. +/// +/// The record is only marked reported on proof of delivery — a non-empty +/// Sentry id — or when the user has opted out, which is a deliberate +/// suppression rather than a failure to retry. Anything else leaves it +/// pending for the next launch. @visibleForTesting -Future flushPendingStartupFailure({Future Function(StartupFailureRecord record)? send}) async { +Future flushPendingStartupFailure({ + Future Function(StartupFailureRecord record)? send, + bool? reporterReady, + bool? crashReportingEnabled, + bool? reportingCompiledIn, +}) async { final record = await StartupDiagnosticsStore.peekPersisted(); if (record == null || record.reported) return; + + final optedIn = crashReportingEnabled ?? SettingsService.instanceOrNull?.read(SettingsService.crashReporting) ?? true; + final canEverReport = reportingCompiledIn ?? _enableSentry; + if (!optedIn || !canEverReport) { + // Deliberate suppression, not a delivery failure. Marking it reported is + // what lets `consumePrevious` eventually drop the file; without this a + // fork build with no DSN, or an opted-out user, would carry the record + // forever. + appLogger.d( + optedIn + ? 'Startup failure not reported: crash reporting is not available in this build' + : 'Startup failure not reported: crash reporting is disabled', + ); + await StartupDiagnosticsStore.markReported(record); + return; + } + + if (!(reporterReady ?? _crashReporterReady)) { + // Kept on disk deliberately: `consumePrevious` retains an unreported + // record so the next launch can try again. + appLogger.d('Startup failure kept for the next launch: crash reporting is not initialised'); + return; + } + try { - await (send ?? _captureStartupFailure)(record); + final id = await (send ?? _captureStartupFailure)(record); + if (id == const SentryId.empty()) { + // A no-op hub, a disabled client, or an event dropped in `beforeSend`. + appLogger.d('Startup failure was not accepted by the crash reporter; keeping it for the next launch'); + return; + } await StartupDiagnosticsStore.markReported(record); } catch (error, stackTrace) { // Leave it unreported so the next launch tries again. @@ -340,10 +396,10 @@ Future flushPendingStartupFailure({Future Function(StartupFailureRec } } -Future _captureStartupFailure(StartupFailureRecord record) async { +Future _captureStartupFailure(StartupFailureRecord record) { // The original error object is long gone by now; the record is the // allowlisted, already-redacted rendering of it. - await Sentry.captureMessage( + return Sentry.captureMessage( 'Startup failed: ${record.headline}', level: SentryLevel.fatal, withScope: (scope) { diff --git a/lib/services/startup_diagnostics.dart b/lib/services/startup_diagnostics.dart index a83b81fc..bc2831e9 100644 --- a/lib/services/startup_diagnostics.dart +++ b/lib/services/startup_diagnostics.dart @@ -1,5 +1,6 @@ import 'dart:convert'; import 'dart:io'; +import 'dart:math'; import 'package:flutter/foundation.dart' show visibleForTesting; import 'package:path/path.dart' as p; @@ -70,6 +71,10 @@ class StartupPhaseException implements Exception { /// record. That matters because `LogRedactionManager`'s registered-value set is /// seeded by `StorageService.onInit`, which runs *inside* the gate — a failure /// at or before that step leaves only the pattern matcher active. +String _newRecordId() => + '${DateTime.now().microsecondsSinceEpoch.toRadixString(16)}-' + '${Random().nextInt(0xFFFFFF).toRadixString(16)}'; + class StartupFailureRecord { StartupFailureRecord({ required this.phase, @@ -81,7 +86,9 @@ class StartupFailureRecord { required this.platform, this.repairable = false, this.reported = false, - }) : message = LogRedactionManager.redact(message), + String? id, + }) : id = id ?? _newRecordId(), + message = LogRedactionManager.redact(message), stackTrace = stackTrace == null ? null : LogRedactionManager.redact(stackTrace); /// Builds a record from a thrown [error]. @@ -129,6 +136,14 @@ class StartupFailureRecord { return offset == null ? message : '$message (at offset $offset)'; } + /// Distinguishes this record from any other, including one written moments + /// later by a retry that also failed. + /// + /// A slow flush can still be sending record A when a retry writes record B; + /// without an identity, marking A reported would overwrite B on disk and + /// lose the newer failure entirely. + final String id; + final StartupPhase? phase; final String errorType; @@ -154,6 +169,7 @@ class StartupFailureRecord { final bool reported; StartupFailureRecord copyWith({bool? reported}) => StartupFailureRecord( + id: id, phase: phase, errorType: errorType, message: message, @@ -190,6 +206,7 @@ class StartupFailureRecord { } Map toJson() => { + 'id': id, 'phase': phase?.id, 'errorType': errorType, 'message': message, @@ -207,6 +224,9 @@ class StartupFailureRecord { final timestamp = DateTime.tryParse(json['timestamp'] as String? ?? ''); if (message is! String || errorType is! String || timestamp == null) return null; return StartupFailureRecord( + // Records written before ids existed fall back to their timestamp, which + // is stable for a given file even if it is not globally unique. + id: json['id'] as String? ?? 'legacy-${timestamp.microsecondsSinceEpoch}', phase: StartupPhase.fromId(json['phase'] as String?), errorType: errorType, message: message, @@ -238,6 +258,15 @@ abstract final class StartupDiagnosticsStore { static StartupFailureRecord? _pending; + /// A crash-report flush that must finish before the record may be consumed. + static Future? _flushInFlight; + + /// The persist started by [record]. Callers launch it unawaited from the + /// failure path, so every later reader has to settle it first or it can land + /// after a peek (losing the report) or after a consume (stranding a stale + /// unreported file). + static Future? _writeInFlight; + /// Record observed during this launch, if any. Set both when a failure is /// recorded and when one written by an earlier launch is consumed. static StartupFailureRecord? get pending => _pending; @@ -254,8 +283,40 @@ abstract final class StartupDiagnosticsStore { /// Best-effort write. A diagnostics failure must never worsen the failure it /// is describing, so every error here is logged and swallowed. - static Future record(StartupFailureRecord failure) async { + static Future record(StartupFailureRecord failure) { + // Published synchronously: the failure screen and the logs banner read it + // straight away, and the disk write below may never land at all. _pending = failure; + return _enqueueWrite(() => _write(failure)); + } + + /// Serializes every mutation of the record file. + /// + /// Writes are launched unawaited from the failure path, and a retry that + /// also fails writes while the first one may still be running. Two + /// concurrent writers to one file can land out of order, and a + /// read-modify-write like [markReported] can interleave with a write and + /// resurrect the record it just superseded. Chaining makes disk order match + /// call order and makes the compare-and-set atomic against writes (#1732). + static Future _enqueueWrite(Future Function() operation) { + final previous = _writeInFlight; + final task = () async { + if (previous != null) { + try { + await previous; + } catch (_) { + // Best effort: an earlier failed write must not block this one. + } + } + await operation(); + }(); + _writeInFlight = task; + return task.whenComplete(() { + if (identical(_writeInFlight, task)) _writeInFlight = null; + }); + } + + static Future _write(StartupFailureRecord failure) async { try { final file = await _file(); if (file == null) return; @@ -266,11 +327,35 @@ abstract final class StartupDiagnosticsStore { } } + /// Waits for an in-flight [record] so readers never race the write. + static Future _settleWrite() async { + final write = _writeInFlight; + if (write == null) return; + try { + await write; + } catch (error, stackTrace) { + appLogger.d('Startup failure record write failed', error: error, stackTrace: stackTrace); + } + } + + /// Registers a crash-report flush so [consumePrevious] cannot delete the + /// record out from under it. + /// + /// The flush is deliberately off the startup critical path, and the success + /// path consumes the record as soon as the gate completes. Without this gate + /// a fast launch could delete the file before the flush had read it, losing + /// the report entirely — the exact failure this whole path exists to + /// prevent (#1732). + static void holdForFlush(Future flush) { + _flushInFlight = flush; + } + /// 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 { + await _settleWrite(); try { final file = await _file(); if (file == null || !await file.exists()) return null; @@ -285,33 +370,77 @@ abstract final class StartupDiagnosticsStore { /// 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); - } + static Future markReported(StartupFailureRecord failure) { + // Queued with the writes: reading and rewriting outside the queue lets a + // concurrent `record` land between the two and be overwritten, losing the + // newer failure. + return _enqueueWrite(() async { + try { + final file = await _file(); + if (file == null || !await file.exists()) return; + // Compare and set: a retry may have failed too and replaced this + // record while the send was in flight. + final current = await _decodeFile(file); + await debugAfterReportedRead?.call(); + if (current == null || current.id != failure.id) { + appLogger.d('Startup failure record was superseded before it could be marked reported'); + return; + } + final updated = failure.copyWith(reported: true); + if (_pending?.id == failure.id) _pending = updated; + 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. + /// Test seam: pauses [markReported] between its read and its write so the + /// compare-and-set can be exercised against a concurrent record. + @visibleForTesting + static Future Function()? debugAfterReportedRead; + + static Future _decodeFile(File file) async { + final decoded = jsonDecode(await file.readAsString()); + if (decoded is! Map) return null; + return StartupFailureRecord.fromJson(decoded.cast()); + } + + /// Loads a record written by an earlier launch, deleting it only once it has + /// been reported. /// - /// Deleting on read stops one stale failure from following the user forever; - /// the value stays in [pending] for the rest of the session so the logs - /// screen can still show it after the user navigates away and back. + /// An unreported record is the crash reporter's only retry: the reporter can + /// be uninitialised, offline, or backed by a no-op hub that accepts events + /// and drops them. Deleting it here would silently end the retry, which is + /// the failure this whole path exists to prevent (#1732). A record that can + /// never be sent — no crash reporting compiled in, or the user opted out — + /// is marked reported by the flush, so nothing lingers indefinitely. + /// + /// Either way the value lands in [pending] so the logs screen can show it + /// for the rest of the session. static Future consumePrevious() async { + await _settleWrite(); + final flush = _flushInFlight; + if (flush != null) { + _flushInFlight = null; + // A failed flush must not block the display path. + try { + await flush; + } catch (error, stackTrace) { + appLogger.d('Startup failure flush failed before consumption', error: error, stackTrace: stackTrace); + } + } try { final file = await _file(); if (file == null || !await file.exists()) return null; - final raw = await file.readAsString(); - await file.delete(); - final decoded = jsonDecode(raw); - if (decoded is! Map) return null; - final record = StartupFailureRecord.fromJson(decoded.cast()); - if (record != null) _pending = record; + final record = await _decodeFile(file); + if (record == null) { + // Unreadable: nothing can be retried or shown, so drop it. + await file.delete(); + return null; + } + if (record.reported) await file.delete(); + _pending = record; return record; } catch (error, stackTrace) { appLogger.d('Could not read a previous startup failure record', error: error, stackTrace: stackTrace); @@ -321,6 +450,7 @@ abstract final class StartupDiagnosticsStore { /// Drops a persisted record without surfacing it in [pending]. static Future clear() async { + await _settleWrite(); _pending = null; try { final file = await _file(); @@ -333,6 +463,9 @@ abstract final class StartupDiagnosticsStore { @visibleForTesting static void resetForTesting() { _pending = null; + _flushInFlight = null; + _writeInFlight = null; + debugAfterReportedRead = null; debugDirectoryOverride = null; } diff --git a/test/database/tvos_database_recovery_test.dart b/test/database/tvos_database_recovery_test.dart index 48603699..1d33d7b7 100644 --- a/test/database/tvos_database_recovery_test.dart +++ b/test/database/tvos_database_recovery_test.dart @@ -467,6 +467,26 @@ void main() { expect(prefs.getString(TvosDatabaseRecoveryStore.manifestKey), contains('committed')); }); + test('a wrong-typed recovery marker behaves like a missing one', () async { + // `reconcile` runs inside `AppDatabase.open`, a fatal startup step, so + // a mistyped marker used to throw a raw TypeError and veto the launch + // outright on a first-class TV target (#1732). An unreadable marker + // tells us nothing, which is the same position as an absent one. + await prefs.setString(TvosDatabaseRecoveryStore.recoveryRequiredKey, 'yes'); + + final result = await open(); + + expect(result.recoveryOutcome, TvosDatabaseRecoveryOutcome.fresh); + expect(prefs.getString(TvosDatabaseRecoveryStore.manifestKey), contains('committed')); + + // The unreadable value is dropped so the next launch starts clean. The + // removal is fire-and-forget, so give it a turn to land. + for (var i = 0; i < 20 && prefs.keys.contains(TvosDatabaseRecoveryStore.recoveryRequiredKey); i++) { + await Future.delayed(Duration.zero); + } + expect(prefs.keys, isNot(contains(TvosDatabaseRecoveryStore.recoveryRequiredKey))); + }); + test('missing database with prior-install evidence requires recovery', () async { await prefs.setString('active_app_profile_id', 'surviving-profile'); final result = await open(); diff --git a/test/services/startup_diagnostics_test.dart b/test/services/startup_diagnostics_test.dart index 15383c64..c8564753 100644 --- a/test/services/startup_diagnostics_test.dart +++ b/test/services/startup_diagnostics_test.dart @@ -1,7 +1,9 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:plezy/main.dart'; import 'package:plezy/services/sensitive_prefs.dart'; import 'package:plezy/services/startup_diagnostics.dart'; @@ -139,19 +141,33 @@ void main() { expect(restored.stackTrace, contains('#0 frame')); }); - test('consuming deletes the file but keeps the record available in-session', () async { - await StartupDiagnosticsStore.record(_record()); + test('consuming a reported record deletes it but keeps it in-session', () async { + final record = _record(); + await StartupDiagnosticsStore.record(record); + await StartupDiagnosticsStore.markReported(record); StartupDiagnosticsStore.resetForTesting(); StartupDiagnosticsStore.debugDirectoryOverride = tempDir; await StartupDiagnosticsStore.consumePrevious(); - // Deleted so one stale failure cannot follow the user forever, but held - // in memory so Settings > Logs can still show and upload it. expect(await File('${tempDir.path}/${StartupDiagnosticsStore.fileName}').exists(), isFalse); expect(StartupDiagnosticsStore.pending, isNotNull); }); + test('consuming an unreported record keeps it on disk for the next launch', () async { + // The crash reporter may have been offline, opted out, or backed by a + // no-op hub. Deleting here would silently end the only retry there is. + await StartupDiagnosticsStore.record(_record()); + StartupDiagnosticsStore.resetForTesting(); + StartupDiagnosticsStore.debugDirectoryOverride = tempDir; + + final consumed = await StartupDiagnosticsStore.consumePrevious(); + + expect(consumed, isNotNull); + expect(await File('${tempDir.path}/${StartupDiagnosticsStore.fileName}').exists(), isTrue); + expect(StartupDiagnosticsStore.pending, isNotNull); + }); + test('consuming nothing yields null', () async { expect(await StartupDiagnosticsStore.consumePrevious(), isNull); expect(StartupDiagnosticsStore.pending, isNull); @@ -196,37 +212,303 @@ void main() { // inline capture goes to a no-op hub and is silently discarded — the // likeliest failure phase producing no telemetry at all (#1732). + final delivered = SentryId.newId(); + 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)); + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (record) async { + sent.add(record); + return delivered; + }, + ); 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)); + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (record) async { + sent.add(record); + return delivered; + }, + ); expect(sent, hasLength(1)); }); - test('leaves the record unreported when the send fails', () async { + test('leaves the record unreported when the send throws', () async { await StartupDiagnosticsStore.record(_record()); - await flushPendingStartupFailure(send: (_) async => throw StateError('offline')); + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (_) async => throw StateError('offline'), + ); // Still pending, so the next launch retries rather than losing it. expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isFalse); }); + test('an empty Sentry id is not proof of delivery', () async { + // A no-op hub — which is what a failed or timed-out init leaves behind — + // accepts the event and returns an empty id without throwing. Treating + // that as success would suppress the record permanently. + await StartupDiagnosticsStore.record(_record()); + + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (_) async => const SentryId.empty(), + ); + + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isFalse); + }); + + test('does not send while the reporter is uninitialised', () async { + await StartupDiagnosticsStore.record(_record()); + final sent = []; + + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: false, + crashReportingEnabled: true, + send: (record) async { + sent.add(record); + return delivered; + }, + ); + + expect(sent, isEmpty); + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isFalse); + }); + + test('an opted-out user is never sent, and never retried', () async { + await StartupDiagnosticsStore.record(_record()); + final sent = []; + + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: false, + send: (record) async { + sent.add(record); + return delivered; + }, + ); + + expect(sent, isEmpty); + // Suppressed deliberately, so it must not be rediscovered every launch. + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isTrue); + }); + test('does nothing when no launch has failed', () async { final sent = []; - await flushPendingStartupFailure(send: (record) async => sent.add(record)); + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (record) async { + sent.add(record); + return delivered; + }, + ); expect(sent, isEmpty); }); + test('consumption waits for a registered flush', () async { + await StartupDiagnosticsStore.record(_record()); + final started = Completer(); + final release = Completer(); + final seen = []; + + StartupDiagnosticsStore.holdForFlush( + flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (record) async { + seen.add(record); + started.complete(); + await release.future; + return delivered; + }, + ), + ); + await started.future; + + // The success path would otherwise delete the file mid-send. + final consumed = StartupDiagnosticsStore.consumePrevious(); + var finished = false; + unawaited(consumed.then((_) => finished = true)); + await Future.delayed(Duration.zero); + expect(finished, isFalse, reason: 'consumption must not race the flush'); + + release.complete(); + expect((await consumed)!.reported, isTrue); + expect(seen, hasLength(1)); + }); + + test('a slow flush never clobbers a newer retry failure', () async { + // Retry also fails while flush A is still sending: marking A reported + // must not write A back over B. + final first = _record(phase: StartupPhase.preferences); + await StartupDiagnosticsStore.record(first); + + final sending = Completer(); + final release = Completer(); + final flush = flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (_) async { + sending.complete(); + await release.future; + return SentryId.newId(); + }, + ); + await sending.future; + + final second = _record(phase: StartupPhase.database, error: StateError('retry also failed')); + await StartupDiagnosticsStore.record(second); + + release.complete(); + await flush; + + final persisted = await StartupDiagnosticsStore.peekPersisted(); + expect(persisted!.id, second.id, reason: 'the newer failure must survive'); + expect(persisted.phase, StartupPhase.database); + expect(persisted.reported, isFalse, reason: 'B was never sent'); + }); + + test('records are individually identifiable', () { + expect(_record().id, isNot(_record().id)); + // copyWith preserves identity, or the compare-and-set above cannot work. + final record = _record(); + expect(record.copyWith(reported: true).id, record.id); + }); + + test('a peek settles an unawaited write first', () async { + // The failure path launches `record()` unawaited, so a fast retry can + // reach the flush before the file exists. + unawaited(StartupDiagnosticsStore.record(_record(phase: StartupPhase.preferences))); + + final peeked = await StartupDiagnosticsStore.peekPersisted(); + + expect(peeked?.phase, StartupPhase.preferences); + }); + + test('a consume settles an unawaited write first', () async { + final record = _record(); + unawaited(StartupDiagnosticsStore.record(record)); + await StartupDiagnosticsStore.markReported(record); + + expect(await StartupDiagnosticsStore.consumePrevious(), isNotNull); + // No stale file left behind by a late write. + expect(await StartupDiagnosticsStore.peekPersisted(), isNull); + }); + + test('concurrent writes land in record order', () async { + // Two unawaited writes to one file: without a queue the older one can + // finish last and overwrite the newer failure. + final first = _record(phase: StartupPhase.preferences); + final second = _record(phase: StartupPhase.database); + unawaited(StartupDiagnosticsStore.record(first)); + unawaited(StartupDiagnosticsStore.record(second)); + + expect((await StartupDiagnosticsStore.peekPersisted())!.id, second.id); + }); + + test('marking reported never resurrects a record a retry superseded', () async { + // Deterministic interleave: pause the compare-and-set between its read + // and its write, let a retry record a newer failure, then release it. + // Outside the write queue the mark would land last and overwrite B. + final first = _record(phase: StartupPhase.preferences); + await StartupDiagnosticsStore.record(first); + + final reading = Completer(); + final release = Completer(); + StartupDiagnosticsStore.debugAfterReportedRead = () { + if (!reading.isCompleted) reading.complete(); + return release.future; + }; + addTearDown(() => StartupDiagnosticsStore.debugAfterReportedRead = null); + + final marking = StartupDiagnosticsStore.markReported(first); + await reading.future; + + final second = _record(phase: StartupPhase.database); + final writingSecond = StartupDiagnosticsStore.record(second); + + release.complete(); + await marking; + await writingSecond; + + final persisted = await StartupDiagnosticsStore.peekPersisted(); + expect(persisted!.id, second.id, reason: 'the newer failure must survive'); + expect(persisted.reported, isFalse, reason: 'the newer failure was never sent'); + }); + + test('a build without crash reporting resolves the record instead of hoarding it', () async { + // No DSN compiled in: nothing will ever send this, so it must not be + // rediscovered on every launch forever. + await StartupDiagnosticsStore.record(_record()); + final sent = []; + + await flushPendingStartupFailure( + reportingCompiledIn: false, + reporterReady: false, + crashReportingEnabled: true, + send: (record) async { + sent.add(record); + return SentryId.newId(); + }, + ); + + expect(sent, isEmpty); + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isTrue); + }); + + test('an undeliverable record survives a successful launch and retries', () async { + // End to end: no-op hub, the gate then succeeds and consumes, and the + // record is still there for the next launch to send. + await StartupDiagnosticsStore.record(_record()); + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (_) async => const SentryId.empty(), + ); + await StartupDiagnosticsStore.consumePrevious(); + + StartupDiagnosticsStore.resetForTesting(); + StartupDiagnosticsStore.debugDirectoryOverride = tempDir; + final sent = []; + await flushPendingStartupFailure( + reportingCompiledIn: true, + reporterReady: true, + crashReportingEnabled: true, + send: (record) async { + sent.add(record); + return SentryId.newId(); + }, + ); + + expect(sent, hasLength(1), reason: 'the next launch must retry it'); + expect((await StartupDiagnosticsStore.peekPersisted())!.reported, isTrue); + }); + test('peeking leaves the record on disk for the display path', () async { await StartupDiagnosticsStore.record(_record());