Fix: Prevent storage wipe and logout on failed account deletion#611
Conversation
📝 WalkthroughWalkthroughThe account deletion promise chain in DangerZoneTab.tsx was reordered to ensure browser storage clears only after the deletion API call succeeds. The error handler moved from before the success handler to after it, guaranteeing storage clearing, logout, and resolution occur as a unified success path following confirmed deletion. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Fixes the account-deletion flow in Settings → Danger Zone so client-side logout and browser storage clearing only occur after the server confirms the account was successfully deleted, preventing accidental logout/storage wipe on API failure.
Changes:
- Reordered the
deleteRequest.del()promise chain so.then()runs only on successful deletion and.catch()handles failures. - Ensured
localStorage.clear(),sessionStorage.clear(), andlogout()are executed only after confirmed account deletion.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/settings/DangerZoneTab.tsx`:
- Around line 37-38: The deletion flow in DangerZoneTab.tsx currently calls
logout().then(); and immediately calls resolve(), which doesn't wait for logout
to finish; change the handler so you await logout() (or chain
logout().then(...)) before calling resolve(), and handle errors by rejecting or
catching and logging so failures during logout don't get ignored—update the code
that invokes logout() and resolve() (the promise resolution logic) to await
logout(), then call resolve(), or call logout().then(resolve).catch(reject) /
log as appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c27065f9-9aa7-4c20-9734-16182ee41262
📒 Files selected for processing (1)
src/modules/settings/DangerZoneTab.tsx
Fixed the promise chain order in
deleteAccountinsideDangerZoneTab. Previously,.catch()was placed before.then(), which caused a subtle but critical bug — when.catch()handles a rejection it returns a resolved promise, so.then()would always execute regardless of whether the API call succeeded or failed.Changes
.catch((error) => reject(error))to after.then()in thedeleteAccountpromise chainlocalStorage.clear(),sessionStorage.clear(), andlogout()now only run after the server confirms successful account deletionThis makes the deletion flow safe: if the API returns an error, the user stays logged in with storage intact and sees the error notification instead of being silently wiped and logged out.
Before:
After:
Summary by CodeRabbit