Skip to content

fix: prevent crash during wallet backup#1092

Draft
jvsena42 wants to merge 3 commits into
release-2.4.0from
fix/backup-repo-crash
Draft

fix: prevent crash during wallet backup#1092
jvsena42 wants to merge 3 commits into
release-2.4.0from
fix/backup-repo-crash

Conversation

@jvsena42

@jvsena42 jvsena42 commented Jul 17, 2026

Copy link
Copy Markdown
Member

This PR fixes a crash that could put the app into a restart loop shortly after launch.

Description

The wallet backup payload was assembled outside of any error handling, so a failure while reading Paykit state escaped the backup coroutine and brought the whole app down. The automatic backup runs shortly after every launch, so an affected wallet crash-looped on startup with no way for the user to recover.

The payload is now built inside the existing failure handling. A failed read marks the backup as failed and retries later, the same way an upload failure already did. This also covers the retry action on the Backup settings screen, which previously crashed when tapped for a failed wallet backup.

The trigger seen in the wild was a stored Paykit session missing capabilities the SDK now requires, which is only reachable if Paykit was turned on from dev settings. The backup path itself is not gated, so any Paykit initialisation failure could take down the app the same way for any user.

Recovering a stale session and broader coroutine error handling are follow-ups (#1093). This PR only stops the crash, so an affected wallet will report a failed wallet backup instead of crashing.

Preview

N/A — no user-facing UI changes.

QA Notes

Manual Tests

  • 1. regression: Fresh launch with wallet activity → wait for automatic backup: Settings → Backup shows wallet backup synced, no crash.
  • 2. regression: Settings → Backup → tap retry on a failed wallet backup: retries without crashing.
  • 3. Dev settings → Paykit enabled with a stale session → launch and wait for automatic backup: app stays up and wallet backup reports failure instead of crash-looping.

Automated Checks

  • Unit tests added: cover the wallet backup marking a failure instead of crashing when the Paykit snapshot fails, in BackupRepoTest.kt. Verified failing before the fix and passing after.
  • CI: standard compile, unit test, and detekt checks run by the PR bot.

@jvsena42

Copy link
Copy Markdown
Member Author

Waiting for upstream branch merge

vssBackupClient.putObject(key = category.name, data = getBackupDataBytes(category))
val data = runSuspendCatching { getBackupDataBytes(category) }
.onFailure { markBackupFailed(category, backupRequired, it) }
.getOrNull() ?: return@withContext

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triggerBackup now infers Unit because this path returns without a value, but it returned Result<VssItem> before this change. That makes a snapshot failure look like successful completion to callers and changes the method signature. Could we return the snapshot failure as a Result and declare the return type explicitly?

stubBackupObservers()

try {
sut.startObservingBackups()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test still passes if startObservingBackups() is removed, so it can pass without calling backupSnapshot(). The test clock is also 1_000, which makes triggerBackup() set required equal to synced instead of leaving a failed backup pending. Could we set the clock after synced, verify backupSnapshot() is called once, assert the status is still required with running = false, and advance another debounce to confirm it is not retried?

@jvsena42

Copy link
Copy Markdown
Member Author

Tested from https://github.com/synonymdev/bitkit-android/actions/runs/29591563331 . My app stopped crashing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants