Improve Turnstile token handling with retry logic and timeouts - #4708
Conversation
Users intermittently saw "Turnstile error: 600010" immediately on a fresh page load. The cause was the background token prefetch in initialize(): on a cold visit Cloudflare occasionally returns a transient 600xxx challenge failure, and getTurnstileToken() reacted to the very first error by tearing down the widget, rejecting, and firing a blocking alert() — defeating Turnstile's own retry and interrupting a user who hadn't even asked to play. On recent loads Turnstile has cached session state and solves instantly, so the error only showed on first load. (The JWT refresh flow is unrelated.) - Enable Turnstile auto-retry (retry: "auto" + retry-interval) and return true from the error-callback so transient failures recover instead of hard-failing; an overall timeout is the backstop. - Make the prefetch swallow errors silently; only surface a failure at join time, when a token is actually needed. - Route the join-time alert through translateText() with an en.json entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J36fjzdE86cSz2WR7FppAM
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughTurnstile token handling now supports automatic retries, timeout control, safe prefetch reuse, and join-time failure handling. English localization adds a verification failure message. ChangesTurnstile resilience
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Turnstile
participant Lobby
Client->>Turnstile: Prefetch or request token
Turnstile-->>Client: Return token or retry failure
Client->>Lobby: Join with resolved token
Client-->>Client: Show verification failure alert and stop
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/client/Main.ts (2)
1046-1083: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the token TTL magic number into a named constant.
3 * 60 * 1000at line 1064 is a hardcoded freshness window living alongside the already-extractedTURNSTILE_RETRY_INTERVAL_MS/TURNSTILE_OVERALL_TIMEOUT_MSmodule constants. Pulling it out keeps all Turnstile timing knobs in one visible place.♻️ Proposed extraction
- const token = await prefetch.catch(() => null); - const tokenTTL = 3 * 60 * 1000; - if (token && Date.now() < token.createdAt + tokenTTL) { + const token = await prefetch.catch(() => null); + if (token && Date.now() < token.createdAt + TURNSTILE_TOKEN_TTL_MS) {And near the other constants:
+// How long a prefetched token stays fresh enough to reuse at join time. +const TURNSTILE_TOKEN_TTL_MS = 3 * 60 * 1000;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/Main.ts` around lines 1046 - 1083, Extract the Turnstile token freshness duration currently written as 3 * 60 * 1000 in getTurnstileToken into a named module-level constant alongside TURNSTILE_RETRY_INTERVAL_MS and TURNSTILE_OVERALL_TIMEOUT_MS, then use that constant in the token expiry check.
1177-1184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDon’t link
error-callbacktruthiness to auto-retry.Cloudflare’s
error-callbackreturn value only controls whether Turnstile logs its default console warning; automatic retry behavior is driven by retry settings/error codes, not returningtrue.Suggested comment fix
- // Returning true tells Turnstile the error was handled by the site and it - // should auto-retry (see `retry: "auto"` above). Don't tear the widget - // down or reject here — many 600xxx codes recover on a subsequent - // attempt, and the overall timeout guarantees we don't wait forever. + // Returning true only tells Turnstile the error was handled here, so it + // skips its own console warning; retrying is already handled by + // `retry: "auto"` above. Don't tear the widget down or reject here — + // many 600xxx codes recover on a subsequent attempt, and the overall + // timeout guarantees we don't wait forever.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/Main.ts` around lines 1177 - 1184, Update the Turnstile "error-callback" handler to stop describing or relying on its return value as enabling automatic retries. Preserve the settled guard and warning, but return the value appropriate for suppressing Turnstile’s default console warning while leaving retry behavior controlled by the existing retry configuration and error-code handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/Main.ts`:
- Around line 312-320: Guard the turnstileTokenPromise prefetch in initialize()
so it is not started for CrazyGames sessions. Reuse the existing CrazyGames
detection used by getTurnstileToken(lobby), while preserving the current
background prefetch and failure handling for other platforms.
---
Nitpick comments:
In `@src/client/Main.ts`:
- Around line 1046-1083: Extract the Turnstile token freshness duration
currently written as 3 * 60 * 1000 in getTurnstileToken into a named
module-level constant alongside TURNSTILE_RETRY_INTERVAL_MS and
TURNSTILE_OVERALL_TIMEOUT_MS, then use that constant in the token expiry check.
- Around line 1177-1184: Update the Turnstile "error-callback" handler to stop
describing or relying on its return value as enabling automatic retries.
Preserve the settled guard and warning, but return the value appropriate for
suppressing Turnstile’s default console warning while leaving retry behavior
controlled by the existing retry configuration and error-code handling.
🪄 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 Plus
Run ID: ee7f4f9d-a122-460e-afe3-25051e416a0f
📒 Files selected for processing (2)
resources/lang/en.jsonsrc/client/Main.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
… widgets On CrazyGames the join-time path always requests a fresh token and never reuses the prefetched one, discarding the prefetch promise without awaiting or cancelling it. If that abandoned prefetch is still rendering/retrying (up to the 30s timeout), a second widget gets rendered into the same #turnstile-container concurrently. Guard the prefetch so it doesn't start on CrazyGames sessions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J36fjzdE86cSz2WR7FppAM
evanpelle
left a comment
evanpelle
left a comment
There was a problem hiding this comment.
from claude:
In the new join-time catch (Main.ts getTurnstileToken(lobby)), the failure path alerts and then returns null, and handleJoinLobby continues into joinLobby(... turnstileToken: null ...). For a non-Steam first join, the server hard-rejects a null token (src/server/JoinVerify.ts:70 → { action: "reject" }). Previously the rejection propagated out of getTurnstileToken(lobby) and aborted the join before joinLobby was called.
So the user now sees "Verification failed. Please refresh and try again." and then the client still opens a WebSocket and attempts a join that is guaranteed to be rejected — likely producing a second, unrelated-looking failure (or a silently dead join modal). Suggestion: rethrow after the alert (restoring the old abort-the-join behavior while keeping the translated message), or return a sentinel that makes handleJoinLobby bail before calling joinLobby.
The join-time catch previously alerted and returned a null token, then let handleJoinLobby continue into joinLobby(). For a non-dev, non-admitted first join the server hard-rejects a missing token (Worker closes the socket with 1002), so the user saw the "Verification failed" alert and then a second, unrelated-looking failure from the doomed connection. Restore the pre-existing abort-the-join behavior: getTurnstileToken(lobby) now lets a genuine failure propagate, and handleJoinLobby catches it, shows the translated message, and returns before opening the connection. Legitimate null tokens (dev / singleplayer) still pass through unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J36fjzdE86cSz2WR7FppAM
|
Good catch — confirmed and fixed in f1586c8. You're right that the catch-and-return- I went with the "make Generated by Claude Code |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/client/Main.ts (2)
1165-1201: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake the 30-second timeout cover script loading too.
The timeout starts only after the script-polling loop. If Turnstile becomes available near the end of that loop, one request can take almost 40 seconds. Start a single deadline before polling and use the remaining time for both script loading and widget execution.
Based on the PR objective’s 30-second overall timeout budget.
Proposed fix
+const deadline = performance.now() + TURNSTILE_OVERALL_TIMEOUT_MS; + -while (typeof window.turnstile === "undefined" && attempts < 100) { - await new Promise((resolve) => setTimeout(resolve, 100)); - attempts++; +while (typeof window.turnstile === "undefined") { + const remaining = deadline - performance.now(); + if (remaining <= 0) throw new Error("Turnstile timed out"); + await new Promise((resolve) => + setTimeout(resolve, Math.min(100, remaining)), + ); } ... -}, TURNSTILE_OVERALL_TIMEOUT_MS); +}, Math.max(0, deadline - performance.now()));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/Main.ts` around lines 1165 - 1201, Update the Turnstile flow surrounding the script-polling loop and widget execution so the 30-second deadline starts before polling begins. Use one shared deadline or remaining-time calculation for both script loading and the execution timeout, ensuring the total request duration never exceeds TURNSTILE_OVERALL_TIMEOUT_MS while preserving the existing retry and cleanup behavior.
1140-1144: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMove the Turnstile constants above
bootstrap().
bootstrap()callsClient.initialize()beforegetTurnstileToken()is defined, so whenwindow.turnstileis already available the prefetch callsgetTurnstileToken()synchronously and hits theTURNSTILE_RETRY_INTERVAL_MSbinding from this module scope. That binding has not been initialized yet, so the prefetched promise rejects and every join falls back to an on-demand token instead of reusing the prefetched one. Put these constants before thebootstrap()invocation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/Main.ts` around lines 1140 - 1144, Move TURNSTILE_RETRY_INTERVAL_MS and TURNSTILE_OVERALL_TIMEOUT_MS above the bootstrap() invocation in Main.ts, ensuring both bindings are initialized before Client.initialize() can synchronously trigger getTurnstileToken().Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/client/Main.ts`:
- Around line 1165-1201: Update the Turnstile flow surrounding the
script-polling loop and widget execution so the 30-second deadline starts before
polling begins. Use one shared deadline or remaining-time calculation for both
script loading and the execution timeout, ensuring the total request duration
never exceeds TURNSTILE_OVERALL_TIMEOUT_MS while preserving the existing retry
and cleanup behavior.
- Around line 1140-1144: Move TURNSTILE_RETRY_INTERVAL_MS and
TURNSTILE_OVERALL_TIMEOUT_MS above the bootstrap() invocation in Main.ts,
ensuring both bindings are initialized before Client.initialize() can
synchronously trigger getTurnstileToken().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cf7e7287-aaa4-43a4-b5e3-4e510a1e3c36
📒 Files selected for processing (1)
src/client/Main.ts
Two issues from review:
- The TURNSTILE_* constants were declared after the synchronous bootstrap()
call. On a warm load (document already interactive and the Turnstile script
already present), initialize() calls getTurnstileToken() synchronously and
reaches render({ "retry-interval": TURNSTILE_RETRY_INTERVAL_MS }) while that
const is still in the temporal dead zone, throwing a ReferenceError. The
prefetch then always rejected, forcing every join onto an on-demand token.
Move the constants above bootstrap so they're initialized first.
- The 30s timeout only started after the script-load poll (up to ~10s), so a
single request could run ~40s. Use one shared deadline for both the
script-load wait and the execution timeout.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J36fjzdE86cSz2WR7FppAM
|
Both CodeRabbit outside-diff findings were valid and are fixed in c338729:
tsc, ESLint, and Prettier all pass locally. Generated by Claude Code |
github-project-automation
Bot
moved this from Development
to Final Review
in OpenFront Release Management
github-project-automation
Bot
moved this from Final Review
to Complete
in OpenFront Release Management
4 participants
Add approved & assigned issue number here:
Resolves #(issue number)
Description:
Users intermittently saw "Turnstile error: 600010" immediately on a fresh page load. The cause was the background token prefetch in
initialize(): on a cold visit Cloudflare occasionally returns a transient600xxxchallenge failure, andgetTurnstileToken()reacted to the very first error by tearing down the widget, rejecting, and firing a blockingalert()— which also defeated Turnstile's own retry and interrupted a user who hadn't even asked to play. On recent loads Turnstile has cached session state and solves instantly, so the error only showed on first load. (The JWT refresh flow is unrelated.)Changes (
src/client/Main.ts, plus oneen.jsonstring):retry: "auto"+retry-intervalto the widget render, and theerror-callbacknow returnstrue(signalling "handled, retry") instead of removing the widget and rejecting on the first blip. An overall 30s timeout (TURNSTILE_OVERALL_TIMEOUT_MS) is the backstop so a token request can never hang.null, so a cold-load blip no longer interrupts a user who hasn't asked to play.translateText("turnstile.verification_failed"), with the string added toen.json.turnstileTokenPromiseis now typed to reflect that it can resolve tonull.User impact
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
jish
🤖 Generated with Claude Code