fix(app): truthful Drive OAuth errors, visible backup-enable failures, frozen backup path (SELF-3934, SELF-3935) - #2278
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0573091a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| error: error instanceof Error ? error.name : 'unknown', | ||
| }); | ||
| // A dismissed sign-in sheet is the user's own doing — no alert for it. | ||
| if (reason !== 'sign_in_cancelled') { |
There was a problem hiding this comment.
Suppress the alert when biometric unlock is cancelled
When the user dismisses the biometric/keychain prompt opened by getOrCreateMnemonic(), authProvider.tsx recognizes the cancellation and rethrows it, but it is not a CloudBackupError; this catch therefore records it as unexpected_error and displays the new failure alert. Thus a routine user cancellation is both reported as a backup failure and followed by the alert that is deliberately suppressed for an OAuth cancellation. Detect the existing keychain cancellation shape here or propagate a typed cancellation reason before deciding to alert.
Useful? React with 👍 / 👎.
…up enable errors, freeze backup path Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5449b69 to
df1dbb0
Compare
Closes SELF-3935. Part of SELF-3934 — its AC2 (GCP Console audit: client IDs per environment, registered SHA-1s vs the Play App Signing certificate, consent-screen status) cannot be done from the repo and stays open on the issue.
Stack: #2272 → #2273 → #2277 → this. Base is
self-3933-icloud-sync-restore; retarget as the stack merges. Combined per reviewer agreement: the two issues touch the same files and share one theme.SELF-3934 — every Drive OAuth error read as "user cancelled"
googleSignIn()caught everythingauthorize()threw and returned null; callers then reported "sign-in cancelled". Android carries 635 of 701 unique restore-failure users, and misconfiguration was invisible inside the cancel bucket.googleSignIn()now returns null only for an actual cancellation, detected two ways (verified against RNAppAuthModule.java): message match (/cancel/i— Android cancel arrives asauthentication_error/ "User cancelled flow") and OAuth codeaccess_denied(the user tapping Deny on Google's consent page — no "cancel" substring, but it IS a user cancel).CloudBackupError('sign_in_failed')with the native code embedded in the message and the original error ascause. Network failures share the cancel code but not the cancel message (authentication_error/ "Network error"), so they now surface as failures — pinned by a test.reason) through the existingCloudBackupErrorReasonunion — no screen changes.SELF-3935 — silent enable failures + fragile backup path
CloudBackupScreen(previouslyconsole.erroronly — ~400 users in 30 days got nothing) now emitsCLOUD_BACKUP_ENABLE_FAILED { reason, error }and shows an alert, matching the disable path's precedent. No alert whenreasonissign_in_cancelled— the user dismissed the sheet themselves. The toggle already stayed off on failure (it flips only after the awaited upload); a test now pins that.'/@selfxyz/mobile-app', no longer derived frompackage.json'sname(which already changed once, fromopenpassport— a rename would silently orphan every backup). Double leading slash removed; the effective resolved path is byte-identical (nativesanitizePathstrips^/+), pinned by a test asserting the exact production path@selfxyz/mobile-app/encrypted-private-key. The iOSsetProviderOptions(AppData)side effect is preserved.CLOUD_BACKUP_ENABLE_FAILED(additive; also added to the jest.setup analytics mock — along withCLOUD_RESTORE_STARTED, which fix(app): surface cloud restore failures to the user and telemetry (SELF-3932) #2273 missed there).Notes for reviewers
CLOUD_BACKUP_DISABLE_STARTEDdouble-fire (button prop + handler); iOS upload has noisCloudAvailablepre-check (cloud-off enable fails asunexpected_errorafter retries);getOrCreateMnemonicreturning null still exits silently after ENABLE_STARTED.Validation
New coverage: googleSignIn cancel/deny → null, coded failures → typed
sign_in_failedwith cause (incl. the network-shares-cancel-code case); CloudBackupScreen success/failure/cancel — alert shown or suppressed, event reasons, toggle untouched on failure; frozen-path pin;sign_in_failedrendering on the recovery screen.Device QA (extends the run sheet in
app/docs/RECOVERY_QA_RUNSHEET.md): Android — cancel the sign-in sheet → "dismissed" copy,reason: sign_in_cancelled; Deny on the consent page → also a cancel (confirms theaccess_deniedassumption); wrong-client-ID build →sign_in_failedwith a code, not "cancelled"; airplane mode →sign_in_failed. Either platform — airplane-mode enable → alert +CLOUD_BACKUP_ENABLE_FAILED, toggle stays off; restore a production-build backup after the path freeze → still found; enable → disable → file removed from the same location.🤖 Generated with Claude Code