fix: DB readonly-rollback recovery + cooperative import cancellation#255
Merged
Conversation
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).
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.
Contributor
There was a problem hiding this comment.
Pull request overview
Addresses two production issues: (1) app startup failure after a cancelled import left SQLite in a hot-journal/readonly-rollback state, and (2) force-cancel import behavior that could kill SQLite mid-transaction. Adds startup recovery UI and introduces cooperative per-dive cancellation via a cancellation token passed through import adapters/services.
Changes:
- Add
ImportCancellationTokenand plumb cooperative cancellation through import wizard notifier + adapters (DC/UDDF/HealthKit/Universal). - Add startup handling for
SQLITE_READONLY*SqliteExceptions, with a recovery flow that attempts RW-open to force rollback of hot journal. - Expand widget/unit test coverage for cancellation UI/state and for readonly-rollback recovery behavior.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/core/presentation/pages/startup_page.dart | Adds recovery states/UI and triggers recovery flow on readonly-family SQLite errors. |
| lib/core/services/database_service.dart | Opens schema-version probe RW; adds hot-journal recovery helper + readonly error classifier. |
| lib/features/dive_import/data/services/uddf_entity_importer.dart | Adds per-dive cancellation polling for UDDF entity import. |
| lib/features/import_wizard/data/adapters/dive_computer_adapter.dart | Adds per-dive cancellation and fingerprint advancement based on processed dives when cancelled. |
| lib/features/import_wizard/data/adapters/healthkit_adapter.dart | Adds per-dive cancellation polling in HealthKit import loop. |
| lib/features/import_wizard/data/adapters/universal_adapter.dart | Passes cancellation token down into the universal import path. |
| lib/features/import_wizard/domain/adapters/import_source_adapter.dart | Extends adapter API to accept optional cancelToken. |
| lib/features/import_wizard/domain/models/import_cancellation_token.dart | Introduces cooperative cancellation token type. |
| lib/features/import_wizard/presentation/pages/unified_import_wizard.dart | Updates close-button behavior to show cancel/confirm dialogs during import. |
| lib/features/import_wizard/presentation/providers/import_wizard_providers.dart | Adds cancellation state/token lifecycle and passes token into adapter imports. |
| lib/features/import_wizard/presentation/widgets/import_progress_step.dart | Adds cancel button + “cancelling” display state in progress UI. |
| test/core/presentation/pages/startup_page_test.dart | Adds widget tests verifying recovery UI routing and recovery button behavior. |
| test/core/services/database_service_schema_version_test.dart | Adds tests for recoverHotJournal and readonly-error classification. |
| test/features/import_wizard/data/adapters/dive_computer_adapter_test.dart | Adds tests for cancellation breaking the loop + fingerprint advancement behavior. |
| test/features/import_wizard/data/adapters/universal_adapter_test.mocks.dart | Regenerated mocks for new cancelToken parameter. |
| test/features/import_wizard/domain/adapters/import_source_adapter_test.dart | Updates adapter test signature to include cancelToken. |
| test/features/import_wizard/domain/models/import_cancellation_token_test.dart | New unit tests for token behavior (initial/after cancel/idempotent). |
| test/features/import_wizard/presentation/pages/unified_import_wizard_test.dart | Updates fake adapter signature to include cancelToken. |
| test/features/import_wizard/presentation/providers/import_wizard_notifier_test.dart | Adds/updates notifier tests to assert cancellation propagation/state. |
| test/features/import_wizard/presentation/providers/import_wizard_notifier_test.mocks.dart | Regenerated mocks for new cancelToken parameter. |
| test/features/import_wizard/presentation/widgets/import_progress_step_test.dart | Adds widget tests for cancel button enabled/disabled states. |
| test/features/import_wizard/presentation/widgets/import_summary_step_test.dart | Updates fake adapter signature to include cancelToken. |
| test/features/import_wizard/presentation/widgets/review_step_pending_test.dart | Updates test adapter signature to include cancelToken. |
| test/features/import_wizard/presentation/widgets/review_step_test.dart | Updates fake adapter signature to include cancelToken. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
- 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.
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.
…ecover 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.
github-project-automation
Bot
moved this from Backlog
to Done
in Submersion Release Tracker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the incident where a user couldn't relaunch the app after cancelling a 300-dive Perdix AI import. The force-cancel killed SQLite mid-transaction, leaving a hot rollback journal, and the next launch hit
SqliteException(776)onPRAGMA user_version.Two independent fixes ship together:
cbc110731dc): DetectsSQLITE_READONLY_ROLLBACK(extended code 776) on launch and offers a "Recover database" action that reopens the DB in write mode to force the hot-journal rollback. If the reopen itself fails, shows a "Recovery did not complete" screen with Try again / Close and diagnostic details. (Automatic backup-restore fallback is out of scope for this PR — tracked as a follow-up; the primary recovery path solves the incident.) Replaces the previous "reinstall or contact support" dead-end.81c6cf6eed5): Adds anImportCancellationTokenpolled at per-dive boundaries inDiveComputerAdapter,UddfEntityImporter, andHealthKitAdapter. The wizard's close button now shows a real "Cancel import?" dialog; already-imported dives are kept. For dive-computer imports, the fingerprint advances only for dives that actually processed, so the remainder can be re-imported next session. Removes the root cause that produced the hot journal in the first place.All 7120+ tests pass;
flutter analyzeclean;dart formatclean.Test plan
SqliteException(776)on startupUniversalAdapterpath) and confirm per-dive break works for all wrapped formats (CSV, XML, SSRF, UDDF)Known limitations (by design)