Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/fix-oauth-prefetch-race.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@crossmint/client-sdk-react-ui": patch
---

Fix OAuth URL prefetch race condition when auth dialog opens before initialization completes.

- Gate prefetch on `jwt == null` instead of `getAuthStatus() === "logged-out"` so the prefetch still runs when the dialog is open during initialization.
- Skip prefetch when `crossmintAuth` is not yet available to avoid fetching with a null client.
- Initialize `isLoadingOauthUrlMap` to `false` so consumers are not stuck in a loading state when the prefetch has not started.
- Strengthen URL validation to reject empty strings in addition to null/undefined.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const useOAuthWindowListener = (oauthUrlMap: OAuthUrlMap, setError: (erro

const prefetchedUrl = oauthUrlMap[provider];
const resolvedUrl = prefetchedUrl || (await crossmintAuth?.getOAuthUrl(provider));
if (resolvedUrl == null) {
if (!resolvedUrl) {
throw new Error("Failed to resolve OAuth URL");
}
baseUrl = new URL(resolvedUrl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function CrossmintAuthProviderContent({
defaultEmail,
}}
>
<OAuthFlowProvider prefetchOAuthUrls={getAuthStatus() === "logged-out" && prefetchOAuthUrls}>
<OAuthFlowProvider prefetchOAuthUrls={baseAuth.jwt == null && prefetchOAuthUrls}>
<AuthWrapper loginWithOAuthRef={loginWithOAuthRef}>
{children}
<AuthFormDialog open={dialogOpen} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function OAuthFlowProvider({
const { setError } = useAuthForm();

const [oauthUrlMap, setOauthUrlMap] = useState<OAuthUrlMap>(initialOAuthUrlMap);
const [isLoadingOauthUrlMap, setIsLoadingOauthUrlMap] = useState(true);
const [isLoadingOauthUrlMap, setIsLoadingOauthUrlMap] = useState(false);
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.

P2 Brief window where OAuth click silently errors during init

Changing the initial value from true to false is correct semantically, but it means the OAuth buttons are clickable during the gap between the dialog opening and crossmintAuth becoming available. In that gap preFetchAndSetOauthUrl returns early (line 50–52) and oauthUrlMap is still empty, so createPopupAndSetupListeners falls through to crossmintAuth?.getOAuthUrl(provider) which returns undefined, triggering the "Failed to resolve OAuth URL" error to the user.

The original true incidentally blocked this window (though it caused a different bug when the prefetch never ran). A small guard — e.g. setting isLoadingOauthUrlMap to true before returning early when crossmintAuth == null and resetting to false — would prevent the premature-click path, or the OAuth buttons themselves could be disabled while crossmintAuth is not yet available.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/ui/react-ui/src/providers/auth/OAuthFlowProvider.tsx
Line: 41

Comment:
**Brief window where OAuth click silently errors during init**

Changing the initial value from `true` to `false` is correct semantically, but it means the OAuth buttons are clickable during the gap between the dialog opening and `crossmintAuth` becoming available. In that gap `preFetchAndSetOauthUrl` returns early (line 50–52) and `oauthUrlMap` is still empty, so `createPopupAndSetupListeners` falls through to `crossmintAuth?.getOAuthUrl(provider)` which returns `undefined`, triggering the "Failed to resolve OAuth URL" error to the user.

The original `true` incidentally blocked this window (though it caused a different bug when the prefetch never ran). A small guard — e.g. setting `isLoadingOauthUrlMap` to `true` before returning early when `crossmintAuth == null` and resetting to `false` — would prevent the premature-click path, or the OAuth buttons themselves could be disabled while `crossmintAuth` is not yet available.

How can I resolve this? If you propose a fix, please make it concise.

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.

Nice try slithering in here with that take 🐍, but this one's a miss!

No consumer actually reads isLoading from the OAuthFlowContext. All OAuth buttons (GoogleSignIn, TwitterSignIn, ContinueWithGoogle) only destructure startOAuthLogin and activeOAuthProvider — they compute their own loading state via activeOAuthProvider === "google" / "twitter". So whether isLoadingOauthUrlMap is true or false has zero effect on button clickability.

The old useState(true) wasn't "incidentally blocking" anything — it was just leaving a stale true value that nobody reads, while silently preventing the loading state from ever resolving when the prefetch was skipped.

The cold-blooded scenario you describe (click during init → error flash) is the exact same behavior as main today via the existing try-catch from 1398b653. Not a regression from this PR. 🦎

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.

You're right, I concede. Both GoogleSignIn and TwitterSignIn only destructure startOAuthLogin and activeOAuthProvider from useOAuthFlowisLoading (which reflects isLoadingOauthUrlMap) is never read by any consumer. Button clickability is independently derived from activeOAuthProvider === "google/twitter", not from this state. The useState(true)useState(false) change is purely a semantic correctness fix with no behavioral difference on button interaction.

The "error during early click" scenario is also pre-existing behavior surfaced through the try/catch, not introduced by this PR. Comment withdrawn.

Tip: You can give me more context with .greptile/rules.md so I get this right next time.


const {
createPopupAndSetupListeners,
Expand All @@ -47,6 +47,9 @@ export function OAuthFlowProvider({
} = useOAuthWindowListener(oauthUrlMap, setError);

const preFetchAndSetOauthUrl = useCallback(async () => {
if (crossmintAuth == null) {
return;
}
setIsLoadingOauthUrlMap(true);
try {
const oauthProviders = (loginMethods || []).filter(
Expand Down
Loading