Skip to content

Backup: Fix - never move onto an occupied path, retry failed writes - #2359

Open
cmgustavo wants to merge 1 commit into
bitpay:developfrom
cmgustavo:chore/fs-backup-move-hardening-01
Open

cmgustavo wants to merge 1 commit into
bitpay:developfrom
cmgustavo:chore/fs-backup-move-hardening-01

Conversation

@cmgustavo

@cmgustavo cmgustavo commented Aug 31, 2026

Copy link
Copy Markdown
Member

Fixes the ENOENT family in src/store/backup/fs-backup.ts: BITPAY-APP-7, -2X, -52, -K, -N and -21. Ruled out with data: not disk space (fails with 163 GB free), not low-end devices (spread across all three device.class buckets), not Android-only.

Two bugs:

  1. The cache directory cache/bitpay/redux can be cleared by the OS at any moment, including between ensureDir() and the write. There was no retry, so every occurrence reported and no backup landed.
  2. iOS moveFile (NSFileManager moveItemAtPath, RNFSManager.m:419) throws instead of overwriting an existing destination; Android renameTo (RNFSManager.java:365) replaces it. Once a rotation failed and left the final file behind, TEMP→FINAL could never land on iOS. BITPAY-APP-21 is that rotation path — all 10 occurrences carry the Backup rotate failed breadcrumb, none carry Backup read final failed.

Changes:

  • Retry the whole ensureDir → write → rotate → move sequence once. Only the second failure logs, reports and rejects.
  • moveOverwriting: attempt the move first, clear the destination only after it fails. Keeps Android's atomic rename and fixes iOS.
  • backupPersistRoot returns the real promise instead of the swallowed one, so index.ts no longer logs success for a failed write.
  • A failed backup re-arms the trigger so the next persist:root write retries, bounded to 3 consecutive failures. rootPersistConfig sets no throttle and redux-persist defaults to 0, so persist:root is written on every slice change — unbounded re-arming would hammer a permanently broken filesystem.
  • Rotation failures report to Sentry once per session: reporting every occurrence is the noise this PR is removing, reporting none would make a permanently frozen .bak invisible.

Test on iOS and Android

Driven by a temporary __DEV__ harness (not part of this diff) that wipes BASE_DIR right after ensureDir() and forces the rotation to throw, once each per app launch. Replacing the redux directory with a regular file does not work: exists() returns true and ensureDir skips the mkdir on both attempts.

Directory wiped between ensureDir() and the write — the retry recreates it and the backup lands:

[Debug] Backed up store to filesystem, triggered by WALLET/UPDATE_WALLET_NAME.

Rotation forced to fail, leaving the final file occupying the destination — this is the iOS bug, and before this PR the move died here:

[Error] Backup rotate failed - dev: forced rotate failure
[Debug] Backed up store to filesystem, triggered by WALLET/UPDATE_WALLET_NAME.

Library/Caches/bitpay/redux holds a fresh persist-root.json in both cases, with no .tmp left behind.

Android

Same two cases, driven by the same harness. With the rotation forced to fail, renameTo overwrites the occupied final file on the first move and the unlink fallback never runs — the asymmetry this PR relies on, and the reason moveOverwriting attempts the move before clearing the destination:

[Debug] Backed up store to filesystem, triggered by WALLET/SET_CUSTOM_TOKENS_MIGRATION_COMPLETE.
[Error] Backup rotate failed - dev: forced rotate failure
[Debug] Backed up store to filesystem, triggered by WALLET/SET_ACCOUNT_EVM_CREATION_MIGRATION_COMPLETE.

/data/data/com.bitpay.wallet/cache/bitpay/redux ends up in the same state as iOS.

@sentry

sentry Bot commented Aug 31, 2026

Copy link
Copy Markdown

Sentry Issue: BITPAY-APP-2X

@cmgustavo
cmgustavo force-pushed the chore/fs-backup-move-hardening-01 branch from c11bf67 to 61ad3e8 Compare August 31, 2026 21:13
@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown

Sentry Issue: BITPAY-APP-7

@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown

Sentry Issue: BITPAY-APP-52

@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown

Sentry Issue: BITPAY-APP-K

@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown

Sentry Issue: BITPAY-APP-N

@cmgustavo
cmgustavo force-pushed the chore/fs-backup-move-hardening-01 branch from 61ad3e8 to d6c33d6 Compare September 2, 2026 20:53
@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown

Sentry Issue: BITPAY-APP-21

@cmgustavo
cmgustavo force-pushed the chore/fs-backup-move-hardening-01 branch from d6c33d6 to ccfbb14 Compare September 9, 2026 20:07
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.

1 participant