Skip to content

fix(verify-email): prevent redirect loop on failed verification#3082

Closed
v1nayG wants to merge 1 commit into
appwrite:mainfrom
v1nayG:fix/1392-verification-alert-messages
Closed

fix(verify-email): prevent redirect loop on failed verification#3082
v1nayG wants to merge 1 commit into
appwrite:mainfrom
v1nayG:fix/1392-verification-alert-messages

Conversation

@v1nayG

@v1nayG v1nayG commented Jun 10, 2026

Copy link
Copy Markdown

Fixes #1392

Both updateVerificationEmail and updateVerificationPhone read the user store after invalidate() to build the success notification. At that point the store already reflects the new state, so the ternary produces the opposite message — showing "unverified" after verifying and vice versa.

Capture the intended verification state before the API call so the notification always matches the action. Also disambiguate the messages to say "email has been verified" vs "phone has been verified" when the user has both.

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes two success notification messages in the user verification toggle UI (updateStatus.svelte). It captures newVerification and label before the async API calls so message content doesn't depend on when the reactive $user store re-settles after invalidate().

  • Phone verification bug fixed: the original $user.phoneVerification ? 'unverified' : 'verified' (no negation) always displayed the opposite action — e.g., "verified" after marking unverified. The fix uses the pre-captured newVerification flag, which is correct in all cases.
  • Email verification refactored: the original !$user.emailVerification ? 'unverified' : 'verified' was functionally correct (the post-invalidation value equals the new value), but the new approach is cleaner and consistent.
  • UX improvement: messages now include "email" / "phone" to distinguish which credential was changed.

Confidence Score: 5/5

Safe to merge — the change is a targeted fix to success notification text with no side-effects on API calls, navigation, or data flow.

The phone-verification success message was always inverted due to a missing negation; the fix is correct and well-scoped. The email-verification refactoring is functionally equivalent to what it replaced and improves clarity. No logic paths outside the notification strings were touched.

No files require special attention. The untouched updateStatus() function in the same file uses a similar post-invalidation store read, but it maps correctly and is not broken.

Important Files Changed

Filename Overview
src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte Fixes inverted phone-verification success message and refactors both verification functions to capture the intended new state before the async API call, eliminating reliance on post-invalidation reactive store values

Reviews (2): Last reviewed commit: "fix(auth): correct verification alert me..." | Re-trigger Greptile

Comment on lines 31 to 36
redirect(303, resolve('/'));
} catch (error) {
addNotification({
type: 'error',
message: error.message
});

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.

P1 Redirect is caught and swallowed on success

In SvelteKit, redirect() works by throwing a special Redirect exception internally. Because the call is now inside the try block, the catch (error) block intercepts it before SvelteKit can handle it. Since the catch does not re-throw, the redirect is silently dropped: the user stays on /verify-email, sees a spurious error toast (with error.message being undefined or an internal Redirect string), and the success toast is also shown — all after a successful verification.

The safest fix is to use a flag and call redirect after the try/catch, or re-throw Redirect instances in catch using SvelteKit's isRedirect helper:

import { redirect, isRedirect } from '@sveltejs/kit';
// ...
} catch (error) {
    if (isRedirect(error)) throw error;
    addNotification({ type: 'error', message: error.message });
}

Alternatively, track success outside the try:

let verified = false;
try {
    await sdk.forConsole.account.updateVerification({ userId: user, secret });
    addNotification({ type: 'success', message: 'Email has been verified', timeout: 10000 });
    verified = true;
} catch (error) {
    addNotification({ type: 'error', message: error.message });
}
if (verified) redirect(303, resolve('/'));

Both updateVerificationEmail and updateVerificationPhone read the user store after invalidation to build the success notification. At that point the store already reflects the new state, so the ternary produces the opposite message — showing 'unverified' after verifying and vice versa.

Capture the intended verification state and display label before the API call so the notification always matches the action the user just performed. Also disambiguate the messages to indicate whether the email or phone was verified when the user has both.
@v1nayG v1nayG force-pushed the fix/1392-verification-alert-messages branch from 07b349e to 98206bf Compare June 10, 2026 05:42
@v1nayG v1nayG closed this Jun 10, 2026
@v1nayG v1nayG deleted the fix/1392-verification-alert-messages branch June 10, 2026 05:54
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.

🐛 Bug Report: Inconsistency in User Verification Status between Email and Phone Number

1 participant