Skip to content

Commit 0d9c7fd

Browse files
ericgriffinericgriffin
authored
fix: DB readonly-rollback recovery + cooperative import cancellation (#255)
* fix(db): recover readonly-rollback databases on startup A user who cancelled a large dive-computer import could no longer launch the app: every startup failed with `SqliteException(776): attempt to write a readonly database` on `PRAGMA user_version`. Root cause: `getStoredSchemaVersion` opened the db with `OpenMode.readOnly`. When a crashed/cancelled transaction had left a hot journal behind, SQLite's automatic rollback needs write access; a read-only open throws SQLITE_READONLY_ROLLBACK (extended code 776) before the first PRAGMA can run. Fix: - Open RW in `getStoredSchemaVersion` so SQLite can auto-recover the journal during the existence probe. - Add `recoverHotJournal` + `isRecoverableReadonlyError` helpers as defense-in-depth, and an explicit recovery screen on the startup page ("Database needs recovery" → "Recover database" button) so users who still hit a readonly-family exception get a one-tap path instead of the actively-harmful "reinstall or contact support" copy. - Generic-error copy updated to make clear the data is still on disk. Adds unit tests for the new DatabaseService helpers and widget tests for all three recovery-UI states (required, recovering, failed). * fix(import): cooperative cancellation via ImportCancellationToken Previously, cancelling a large import (e.g. 300-dive Perdix download) showed a "cannot be cancelled" dialog and the only option was to force-kill the app. Killing mid-transaction left SQLite with a hot rollback journal, which caused the next launch to hit SQLITE_READONLY_ROLLBACK (776) on PRAGMA user_version. The wizard now offers a real Cancel button. Cancellation is polled cooperatively at per-dive boundaries in DiveComputerAdapter, UddfEntityImporter, and HealthKitAdapter, so the current dive finishes writing to its own transaction before the loop breaks. Already-imported dives are kept. The fingerprint advances only for dives that were actually processed, so the remainder can be re-imported next session. * fix(import): address PR #255 review feedback - Remove dead importedDives variable in DiveComputerAdapter that was only appended to, never read. - Reset isCancellationRequested in the wizard's finally block so a second import starts fresh instead of inheriting the stale flag. - Localize "Cancel import" and "Cancelling..." on the progress step via two new l10n keys (settings_import_cancelButton, settings_import_cancelling) across all 11 supported locales. * test: cover import cancel dialog + DB recovery-failed UI Raise PR #255 patch coverage from 66% toward 85% by adding tests for the branches introduced by the cancel/recovery work: - UnifiedImportWizard cancel dialog (4 tests): confirm dialog opens, Keep importing dismisses without cancelImport, Cancel import fires cancelImport, already-cancelling notice renders instead of the confirm dialog. - ImportProgressStep cancel button wires through to cancelImport. - StartupPage recoveryFailed UI: catch-block routing, Try again re-invokes _runRecovery, Close invokes closeAppOverride. Adds two @VisibleForTesting hooks to UnifiedImportWizard (initialPageOverride, notifierFactoryOverride) so tests can jump to the import-progress page and inject a spy notifier into the widget's inner ProviderScope without driving the full bundle/import pipeline. * fix: clear stale error in _runRecovery and show specific message on recover failure When `DatabaseService.recoverHotJournal` returns false, the recoveryFailed UI now surfaces a concrete message ("SQLite could not reopen the database to roll back the interrupted transaction.") instead of leaving whatever text `_errorMessage` last held. Also clears `_errorMessage` on entry to the `recovering` state so a second attempt cannot leak text from the previous failure into the next UI. Adds a test that writes invalid SQLite bytes (0xAB) to exercise the !recovered branch: the handle opens but `PRAGMA user_version` throws SQLITE_NOTADB inside recoverHotJournal, which catches and returns false. Addresses Copilot review comment on PR #255.
1 parent 5381574 commit 0d9c7fd

47 files changed

Lines changed: 1609 additions & 32 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎lib/core/presentation/pages/startup_page.dart‎

Lines changed: 217 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:flutter/material.dart';
44
import 'package:flutter/services.dart';
55
import 'package:package_info_plus/package_info_plus.dart';
66
import 'package:shared_preferences/shared_preferences.dart';
7+
import 'package:sqlite3/sqlite3.dart' as sqlite3;
78

89
import 'package:submersion/core/database/database.dart';
910
import 'package:submersion/core/database/database_version_exception.dart';
@@ -43,6 +44,9 @@ enum _StartupState {
4344
backingUp,
4445
migrating,
4546
backupFailed,
47+
recoveryRequired,
48+
recovering,
49+
recoveryFailed,
4650
ready,
4751
error,
4852
}
@@ -99,6 +103,7 @@ class _StartupWrapperState extends State<StartupWrapper>
99103
int _dbVersion = 0;
100104
int _appVersion = 0;
101105
BackupFailedException? _backupError;
106+
sqlite3.SqliteException? _readonlyError;
102107

103108
/// Drives the dissolve of the splash layer over the mounted app beneath.
104109
/// Forward-only; starts when _state first reaches ready.
@@ -199,6 +204,27 @@ class _StartupWrapperState extends State<StartupWrapper>
199204
_appVersion = e.appVersion;
200205
});
201206
}
207+
} on sqlite3.SqliteException catch (e) {
208+
if (DatabaseService.isRecoverableReadonlyError(e)) {
209+
debugPrint(
210+
'Startup hit SQLITE_READONLY (code ${e.extendedResultCode}); '
211+
'offering hot-journal recovery.',
212+
);
213+
if (mounted) {
214+
setState(() {
215+
_state = _StartupState.recoveryRequired;
216+
_readonlyError = e;
217+
});
218+
}
219+
} else {
220+
debugPrint('FATAL: App initialization failed: $e');
221+
if (mounted) {
222+
setState(() {
223+
_state = _StartupState.error;
224+
_errorMessage = '$e';
225+
});
226+
}
227+
}
202228
} catch (e) {
203229
debugPrint('FATAL: App initialization failed: $e');
204230
if (mounted) {
@@ -210,6 +236,42 @@ class _StartupWrapperState extends State<StartupWrapper>
210236
}
211237
}
212238

239+
/// Attempt to recover the database from a hot-journal-readonly error by
240+
/// reopening in read-write mode (which forces SQLite to finish the
241+
/// rollback), then retry initialization from the top.
242+
Future<void> _runRecovery() async {
243+
if (!mounted) return;
244+
setState(() {
245+
_state = _StartupState.recovering;
246+
// Clear any stale text from a prior failed attempt so the
247+
// recoveryFailed UI reflects only the current reason.
248+
_errorMessage = '';
249+
});
250+
try {
251+
final dbPath = await widget.locationService.getDatabasePath();
252+
final recovered = DatabaseService.recoverHotJournal(dbPath);
253+
if (!recovered) {
254+
if (mounted) {
255+
setState(() {
256+
_state = _StartupState.recoveryFailed;
257+
_errorMessage =
258+
'SQLite could not reopen the database to roll back the '
259+
'interrupted transaction.';
260+
});
261+
}
262+
return;
263+
}
264+
await _runInitialization();
265+
} catch (e) {
266+
if (mounted) {
267+
setState(() {
268+
_state = _StartupState.recoveryFailed;
269+
_errorMessage = '$e';
270+
});
271+
}
272+
}
273+
}
274+
213275
Future<void> _initializeServices() async {
214276
void onProgress(int currentStep, int totalSteps) {
215277
if (mounted) {
@@ -390,7 +452,10 @@ class _StartupWrapperState extends State<StartupWrapper>
390452
debugShowCheckedModeBanner: false,
391453
home:
392454
(_state == _StartupState.error ||
393-
_state == _StartupState.backupFailed)
455+
_state == _StartupState.backupFailed ||
456+
_state == _StartupState.recoveryRequired ||
457+
_state == _StartupState.recovering ||
458+
_state == _StartupState.recoveryFailed)
394459
? Scaffold(
395460
key: const ValueKey('error'),
396461
backgroundColor: backgroundColor,
@@ -497,6 +562,12 @@ class _StartupWrapperState extends State<StartupWrapper>
497562
);
498563
}
499564

565+
if (_state == _StartupState.recoveryRequired ||
566+
_state == _StartupState.recovering ||
567+
_state == _StartupState.recoveryFailed) {
568+
return _buildRecoveryContent(textColor, subtitleColor);
569+
}
570+
500571
if (_isVersionMismatch) {
501572
return Padding(
502573
padding: const EdgeInsets.all(24),
@@ -561,7 +632,8 @@ class _StartupWrapperState extends State<StartupWrapper>
561632
const SizedBox(height: 16),
562633
Text(
563634
'Try restarting the app. If this persists, '
564-
'reinstall or contact support.',
635+
'contact support — your data is still on disk and does not '
636+
'require a reinstall.',
565637
style: TextStyle(fontSize: 14, color: subtitleColor),
566638
textAlign: TextAlign.center,
567639
),
@@ -571,4 +643,147 @@ class _StartupWrapperState extends State<StartupWrapper>
571643
),
572644
);
573645
}
646+
647+
Widget _buildRecoveryContent(Color textColor, Color subtitleColor) {
648+
if (_state == _StartupState.recovering) {
649+
return Padding(
650+
padding: const EdgeInsets.all(24),
651+
child: Column(
652+
mainAxisSize: MainAxisSize.min,
653+
children: [
654+
const SizedBox(
655+
width: 64,
656+
height: 64,
657+
child: CircularProgressIndicator(),
658+
),
659+
const SizedBox(height: 24),
660+
Text(
661+
'Recovering database...',
662+
style: TextStyle(
663+
fontSize: 20,
664+
fontWeight: FontWeight.bold,
665+
color: textColor,
666+
),
667+
textAlign: TextAlign.center,
668+
),
669+
const SizedBox(height: 16),
670+
Text(
671+
'Rolling back the interrupted transaction. This usually '
672+
'takes a few seconds.',
673+
style: TextStyle(fontSize: 14, color: subtitleColor),
674+
textAlign: TextAlign.center,
675+
),
676+
],
677+
),
678+
);
679+
}
680+
681+
if (_state == _StartupState.recoveryFailed) {
682+
final details = _errorMessage.isNotEmpty
683+
? _errorMessage
684+
: (_readonlyError?.toString() ?? '');
685+
return Padding(
686+
padding: const EdgeInsets.all(24),
687+
child: Column(
688+
mainAxisSize: MainAxisSize.min,
689+
children: [
690+
const Icon(Icons.error_outline, size: 64, color: Colors.orange),
691+
const SizedBox(height: 24),
692+
Text(
693+
'Recovery did not complete',
694+
style: TextStyle(
695+
fontSize: 20,
696+
fontWeight: FontWeight.bold,
697+
color: textColor,
698+
),
699+
textAlign: TextAlign.center,
700+
),
701+
const SizedBox(height: 16),
702+
Text(
703+
'The database could not be rolled back automatically. Your '
704+
'data is still on disk; contact support before reinstalling '
705+
'so we can help you recover it.',
706+
style: TextStyle(fontSize: 14, color: subtitleColor),
707+
textAlign: TextAlign.center,
708+
),
709+
if (details.isNotEmpty) ...[
710+
const SizedBox(height: 12),
711+
Text(
712+
details,
713+
style: TextStyle(
714+
fontSize: 12,
715+
color: subtitleColor,
716+
fontFamily: 'monospace',
717+
),
718+
textAlign: TextAlign.center,
719+
),
720+
],
721+
const SizedBox(height: 24),
722+
Wrap(
723+
spacing: 12,
724+
children: [
725+
OutlinedButton(
726+
onPressed: _runRecovery,
727+
child: const Text('Try again'),
728+
),
729+
FilledButton(onPressed: _closeApp, child: const Text('Close')),
730+
],
731+
),
732+
],
733+
),
734+
);
735+
}
736+
737+
// recoveryRequired
738+
final code = _readonlyError?.extendedResultCode;
739+
return Padding(
740+
padding: const EdgeInsets.all(24),
741+
child: Column(
742+
mainAxisSize: MainAxisSize.min,
743+
children: [
744+
const Icon(Icons.build_circle_outlined, size: 64, color: Colors.blue),
745+
const SizedBox(height: 24),
746+
Text(
747+
'Database needs recovery',
748+
style: TextStyle(
749+
fontSize: 20,
750+
fontWeight: FontWeight.bold,
751+
color: textColor,
752+
),
753+
textAlign: TextAlign.center,
754+
),
755+
const SizedBox(height: 16),
756+
Text(
757+
'A previous session was interrupted while writing to the '
758+
'database. Your data is still on disk; we just need to finish '
759+
'rolling back the cancelled change before the app can open.',
760+
style: TextStyle(fontSize: 14, color: subtitleColor),
761+
textAlign: TextAlign.center,
762+
),
763+
if (code != null) ...[
764+
const SizedBox(height: 12),
765+
Text(
766+
'SQLite code $code',
767+
style: TextStyle(
768+
fontSize: 12,
769+
color: subtitleColor,
770+
fontFamily: 'monospace',
771+
),
772+
textAlign: TextAlign.center,
773+
),
774+
],
775+
const SizedBox(height: 24),
776+
FilledButton(
777+
onPressed: _runRecovery,
778+
child: const Text('Recover database'),
779+
),
780+
const SizedBox(height: 8),
781+
TextButton(
782+
onPressed: _closeApp,
783+
child: const Text('Close without recovering'),
784+
),
785+
],
786+
),
787+
);
788+
}
574789
}

‎lib/core/services/database_service.dart‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,16 @@ class DatabaseService {
196196
/// Reads the stored schema version from a database file without opening it
197197
/// through Drift. Returns null if the file does not exist, or the integer
198198
/// PRAGMA user_version value otherwise.
199+
///
200+
/// Opens in read-write mode (not read-only) so SQLite can automatically
201+
/// roll back any hot journal left behind by a previous crash. A read-only
202+
/// open on a db with a pending rollback throws SQLITE_READONLY_ROLLBACK
203+
/// (extended code 776) before even the first PRAGMA can execute.
199204
static int? getStoredSchemaVersion(String dbPath) {
200205
final file = File(dbPath);
201206
if (!file.existsSync()) return null;
202207

203-
final db = sqlite3.sqlite3.open(dbPath, mode: sqlite3.OpenMode.readOnly);
208+
final db = sqlite3.sqlite3.open(dbPath, mode: sqlite3.OpenMode.readWrite);
204209
try {
205210
final result = db.select('PRAGMA user_version');
206211
if (result.isEmpty) return null;
@@ -210,6 +215,38 @@ class DatabaseService {
210215
}
211216
}
212217

218+
/// Force SQLite to complete any pending hot-journal rollback on [dbPath].
219+
///
220+
/// Opens the file in read-write mode — the very act of opening triggers
221+
/// SQLite's automatic recovery of a hot journal. Returns true if the file
222+
/// opened cleanly (recovery either wasn't needed or succeeded), false if
223+
/// the journal could not be rolled back (file still read-only, on a
224+
/// read-only volume, etc.).
225+
///
226+
/// Safe to call on a file without a hot journal: it simply no-ops.
227+
static bool recoverHotJournal(String dbPath) {
228+
final file = File(dbPath);
229+
if (!file.existsSync()) return true;
230+
try {
231+
final db = sqlite3.sqlite3.open(dbPath, mode: sqlite3.OpenMode.readWrite);
232+
try {
233+
db.select('PRAGMA user_version');
234+
} finally {
235+
db.dispose();
236+
}
237+
return true;
238+
} on sqlite3.SqliteException {
239+
return false;
240+
}
241+
}
242+
243+
/// True if [error] is a [sqlite3.SqliteException] in the SQLITE_READONLY
244+
/// family (primary result code 8) — typically SQLITE_READONLY_ROLLBACK
245+
/// (776) after a cancelled transaction left a hot journal behind.
246+
static bool isRecoverableReadonlyError(Object error) {
247+
return error is sqlite3.SqliteException && error.resultCode == 8;
248+
}
249+
213250
/// Resolve the database path using location service or default
214251
Future<String> _resolveDatabasePath() async {
215252
if (_locationService != null) {

‎lib/features/dive_import/data/services/uddf_entity_importer.dart‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import 'package:submersion/features/equipment/data/repositories/equipment_reposi
2828
import 'package:submersion/features/equipment/data/repositories/equipment_set_repository_impl.dart';
2929
import 'package:submersion/features/equipment/domain/entities/equipment_item.dart';
3030
import 'package:submersion/features/equipment/domain/entities/equipment_set.dart';
31+
import 'package:submersion/features/import_wizard/domain/models/import_cancellation_token.dart';
3132
import 'package:submersion/features/import_wizard/domain/models/import_phase.dart';
3233
import 'package:submersion/features/tags/data/repositories/tag_repository.dart';
3334
import 'package:submersion/features/tags/domain/entities/tag.dart';
@@ -216,13 +217,18 @@ class UddfEntityImporter {
216217
///
217218
/// Only entities at indices present in [selections] are imported.
218219
/// Reports progress via [onProgress] callback.
220+
///
221+
/// If [cancelToken] is non-null, the dive-import loop polls
222+
/// [ImportCancellationToken.isCancelled] between each dive and returns the
223+
/// partial result already persisted when cancellation is observed.
219224
Future<UddfEntityImportResult> import({
220225
required UddfImportResult data,
221226
required UddfImportSelections selections,
222227
required ImportRepositories repositories,
223228
required String diverId,
224229
bool retainSourceDiveNumbers = false,
225230
ImportProgressCallback? onProgress,
231+
ImportCancellationToken? cancelToken,
226232
}) async {
227233
final now = DateTime.now();
228234

@@ -350,6 +356,7 @@ class UddfEntityImporter {
350356
retainSourceDiveNumbers: retainSourceDiveNumbers,
351357
now: now,
352358
onProgress: onProgress,
359+
cancelToken: cancelToken,
353360
);
354361

355362
return UddfEntityImportResult(
@@ -943,6 +950,7 @@ class UddfEntityImporter {
943950
bool retainSourceDiveNumbers = false,
944951
required DateTime now,
945952
ImportProgressCallback? onProgress,
953+
ImportCancellationToken? cancelToken,
946954
}) async {
947955
if (selected.isEmpty) return const _DiveImportResult(0, 0);
948956
onProgress?.call(ImportPhase.dives, 0, selected.length);
@@ -965,6 +973,8 @@ class UddfEntityImporter {
965973
: await repos.diveRepository.getNextDiveNumber(diverId: diverId);
966974

967975
for (final i in sortedSelected) {
976+
if (cancelToken?.isCancelled ?? false) break;
977+
968978
final diveData = items[i];
969979

970980
// Build profile (include setpoint/ppO2 sensor readings)

0 commit comments

Comments
 (0)