Merged
Conversation
- Firebase is blocking github login - Added better error message - Error handling now pretty and better
fix: Prettier error handling
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughEnhanced Google and GitHub sign-in flows to detect account credential conflicts and generate user-friendly error messages. When an email is already associated with different sign-in methods, the system now retrieves and displays those existing methods instead of showing a generic error. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
✅ Deploy Preview for system-craft-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/firebase/auth.ts (2)
63-65: Consider a shared OAuth popup helper to remove duplication.The Google/GitHub conflict branches are now structurally identical; extracting a shared helper will reduce drift risk.
♻️ Example direction
+const signInWithOAuthPopup = async ( + provider: GoogleAuthProvider | GithubAuthProvider, + attemptedProvider: "Google" | "GitHub", + label: "Google" | "GitHub" +) => { + if (!auth) throw new Error("Firebase Auth is not initialized."); + try { + const res = await signInWithPopup(auth, provider); + return res.user; + } catch (error) { + const authError = error as AuthError; + console.error(`${label} sign-in error:`, { + code: authError.code, + message: authError.message, + }); + if (authError.code === "auth/account-exists-with-different-credential") { + throw new Error(await buildProviderConflictMessage(authError, attemptedProvider)); + } + throw new Error(authError.message || `Failed to sign in with ${label}`); + } +}; + export const signInWithGoogle = async () => { - if (!auth) throw new Error("Firebase Auth is not initialized."); - try { - const res = await signInWithPopup(auth, googleProvider); - return res.user; - } catch (error) { - const authError = error as AuthError; - console.error("Google sign-in error:", { - code: authError.code, - message: authError.message, - }); - if (authError.code === "auth/account-exists-with-different-credential") { - throw new Error(await buildProviderConflictMessage(authError, "Google")); - } - throw new Error(authError.message || "Failed to sign in with Google"); - } + return signInWithOAuthPopup(googleProvider, "Google", "Google"); }; export const signInWithGitHub = async () => { - if (!auth) throw new Error("Firebase Auth is not initialized."); - try { - const res = await signInWithPopup(auth, githubProvider); - return res.user; - } catch (error) { - const authError = error as AuthError; - console.error("GitHub sign-in error:", { - code: authError.code, - message: authError.message, - }); - if (authError.code === "auth/account-exists-with-different-credential") { - throw new Error(await buildProviderConflictMessage(authError, "GitHub")); - } - throw new Error(authError.message || "Failed to sign in with GitHub"); - } + return signInWithOAuthPopup(githubProvider, "GitHub", "GitHub"); };Also applies to: 81-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/firebase/auth.ts` around lines 63 - 65, The two branches handling provider-conflict errors (the authError.code === "auth/account-exists-with-different-credential" blocks at the Google and GitHub sign-in sites) are identical; extract a shared helper (e.g., handleProviderConflict or buildAndThrowProviderConflict) that takes the authError and provider name and throws new Error(await buildProviderConflictMessage(...)); then replace both duplicated branches to call that helper from the Google and GitHub sign-in code paths so the logic lives in one place (reference buildProviderConflictMessage and the authError checks).
34-49: Extract the repeated fallback message constant.The same fallback string appears 3 times, which makes future copy edits error-prone.
♻️ Proposed cleanup
async function buildProviderConflictMessage(authError: AuthError, attemptedProvider: "Google" | "GitHub") { const email = authError.customData?.email; + const fallbackMessage = + `An account already exists with this email. Sign in using the original method first, then connect ${attemptedProvider} later.`; if (!auth || !email) { - return `An account already exists with this email. Sign in using the original method first, then connect ${attemptedProvider} later.`; + return fallbackMessage; } try { const methods = await fetchSignInMethodsForEmail(auth, email); if (methods.length === 0) { - return `An account already exists with this email. Sign in using the original method first, then connect ${attemptedProvider} later.`; + return fallbackMessage; } const formattedMethods = methods.map(formatProviderName).join(" or "); return `This email is already registered with ${formattedMethods}. Sign in using ${formattedMethods} first, then connect ${attemptedProvider} later.`; } catch { - return `An account already exists with this email. Sign in using the original method first, then connect ${attemptedProvider} later.`; + return fallbackMessage; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/firebase/auth.ts` around lines 34 - 49, Extract the repeated fallback string into a single constant (e.g., FALLBACK_EMAIL_EXISTS_MSG) and replace the three inline occurrences with that constant; update the logic around fetchSignInMethodsForEmail, methods.map(formatProviderName) and the catch block to reference the new constant while preserving the dynamic attemptedProvider interpolation where needed (compose the final message by either returning the constant directly or using a template that inserts attemptedProvider). Ensure the constant is declared near the top of the module and used in the early-return, methods.length === 0 branch, and the catch branch to avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/firebase/auth.ts`:
- Around line 63-65: The two branches handling provider-conflict errors (the
authError.code === "auth/account-exists-with-different-credential" blocks at the
Google and GitHub sign-in sites) are identical; extract a shared helper (e.g.,
handleProviderConflict or buildAndThrowProviderConflict) that takes the
authError and provider name and throws new Error(await
buildProviderConflictMessage(...)); then replace both duplicated branches to
call that helper from the Google and GitHub sign-in code paths so the logic
lives in one place (reference buildProviderConflictMessage and the authError
checks).
- Around line 34-49: Extract the repeated fallback string into a single constant
(e.g., FALLBACK_EMAIL_EXISTS_MSG) and replace the three inline occurrences with
that constant; update the logic around fetchSignInMethodsForEmail,
methods.map(formatProviderName) and the catch block to reference the new
constant while preserving the dynamic attemptedProvider interpolation where
needed (compose the final message by either returning the constant directly or
using a template that inserts attemptedProvider). Ensure the constant is
declared near the top of the module and used in the early-return, methods.length
=== 0 branch, and the catch branch to avoid duplication.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit