feat(auth): onUpgradeFailure callback when account linking fails#1355
feat(auth): onUpgradeFailure callback when account linking fails#1355russellwheatley wants to merge 11 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an onUpgradeFailure callback to the anonymous upgrade behaviors, allowing for custom error handling and suppression during the linking process. The changes include new type definitions, a centralized error handling utility, and updated logic for both credential and provider linking handlers. A critical issue was identified in the provider linking implementation where the unconditional removal of the user ID from local storage in a finally block would disrupt the redirect flow.
|
I think I'd like to get some integration tests setup for this before going further with this PR as there are some changes to core behaviour. |
This comment was marked as outdated.
This comment was marked as outdated.
|
plan here:
|
Verify existing anonymous user auto-upgrade behavior is unchanged. Verify the new onUpgradeFailure callback fires correctly on account-linking conflicts. Verify returning "handled" from the callback suppresses the default FirebaseUI error UI. Verify anonymous user upgrade state is preserved correctly across provider redirect flows.
Global setup dropped the execSync import while removing the unused runBuildPackages() helper, but ensureFirebaseToolsCached() still calls execSync. The resulting ReferenceError was swallowed by a bare catch and rethrown as a misleading "Auth emulator cannot start" error, failing the ui-e2e CI job before any tests could run.
…ures getRedirectResult() rejections (e.g. auth/email-already-in-use when linking a provider via providerRedirectStrategy) previously bypassed onUpgradeFailure entirely and always fell through to the default FirebaseUI error, so redirect-based conflicts couldn't be customized or suppressed like popup/credential ones. Give redirect behaviors a chance to handle the getRedirectResult() error in config.ts before the default handler runs, and wire autoUpgradeAnonymousUserRedirectHandler to invoke onUpgradeFailure (with oldUserId and a best-effort recovered credential) and signal "handled" back up when the callback suppresses the error. Add unit coverage for the new redirect-failure paths and e2e coverage (react + angular) for a redirect-based linking conflict, both shown and suppressed.
mikehardy
left a comment
mikehardy
left a comment
There was a problem hiding this comment.
LGTM - I had a personal curiosity if these test files - which are living next to the implementation files were leaking into releases in any way - verified they are not. Code all fine and all the testing is excellent. Really happy to see the e2e stuff working out
2 participants
onUpgradeFailurecallback for anonymous user auto-upgrade failures."handled".onUpgradeFailureto redirect-completed linking failures (e.g.auth/email-already-in-usefromgetRedirectResult()), which previously bypassed the callback and always showed the default error.closes #1354
Below demonstrates
onUpgradeFailurefiring when auto upgrade anonymous user fails and also returns "handled" so error message does not appear in the UI.Screen.Recording.2026-05-07.at.16.47.44.mov