User-friendly error on ws close 1002: translate most + explain ways to fix turnstile token invalid/rejected#3863
User-friendly error on ws close 1002: translate most + explain ways to fix turnstile token invalid/rejected#3863VariableVince wants to merge 7 commits intomainfrom
Conversation
… displaying raw error to user
WalkthroughStandardizes WebSocket close-reason codes in Worker, adds English-only translation helper and i18n-aware client handling, and inserts corresponding English translation entries for worker errors. ChangesWorker error codes + client i18n handling
Sequence Diagram(s)sequenceDiagram
participant Client
participant Transport
participant Worker
participant I18n as "TranslationService"
Client->>Transport: open WebSocket
Transport->>Worker: handshake / token
Worker->>Worker: validate token, check account, locate game
alt failure (token/permission/game)
Worker->>Transport: send WSError(payload) and close
Transport->>I18n: translateText(reason) and getEnglishText(reason)
I18n-->>Transport: localized message + english fallback
Transport->>Client: show alert(localized + fallback + tips)
else success
Worker->>Transport: accept connection / join
Transport->>Client: normal flow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
evanpelle
left a comment
evanpelle
left a comment
There was a problem hiding this comment.
instead can we pass a translation key? and then the client can translate it
… refused: " followed by translated error. Cosmetic.reason isn't translated, doesn't seem like going another level deeper really helps. Add translated tips if turnstile is invalid. Append English error for screenshots so devs understand what error user gets.
Yes. I've now made it so it translates all 1002 returned by the worker. Always starts with translation for "connection refused: " followed by translated error. Add translated tips if turnstile is invalid. Append English error for screenshots so devs understand what error user gets. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… translation for it, like "ClientJoinMessageSchema", "WS_ERR_UNEXPECTED_RSV_1" or cosmetics.reason. Fix translation key name for "forbidden" in en.json
VariableVince
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/client/Transport.ts (1)
383-390:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard empty close reasons before building
worker_error.${event.reason}.
CloseEvent.reasoncan be empty. On that path Line 383 becomesworker_error.and Lines 388-390 fall back to an empty suffix, so the alert degrades to justConnection refused:instead of giving the user anything actionable.MDN WebSocket CloseEvent.reason: can the reason be an empty string when no close reason was provided?💡 Minimal fix
- const errorKey = `worker_error.${event.reason}`; + const rawReason = event.reason.trim(); + const errorKey = rawReason ? `worker_error.${rawReason}` : null; let alertMsg = `${translateText(connRefusedKey)}: `; - const translatedError = translateText(errorKey); + const translatedError = errorKey + ? translateText(errorKey) + : translateText("troubleshooting.unknown"); - if (translatedError === errorKey) { + if (!errorKey || translatedError === errorKey) { // No translation key in error.reason or no translation or default English found - alertMsg += `${event.reason}`; + alertMsg += rawReason || translateText("troubleshooting.unknown"); } else { alertMsg += translatedError; // Add tips if token invalid - if (event.reason === "turnstile_invalid") { + if (rawReason === "turnstile_invalid") { alertMsg += `\n${translateText("worker_error.turnstile_fix_tips")}`; }🤖 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/Transport.ts` around lines 383 - 390, Guard against an empty CloseEvent.reason before constructing errorKey; if event.reason is falsy or only whitespace, avoid building `worker_error.${event.reason}` and instead use a fallback key or include a descriptive fallback string (e.g., "unknown" or the numeric close code) when calling translateText. Update the code around the errorKey creation and the logic that appends to alertMsg (referencing the variables `event`, `errorKey`, `translateText`, `connRefusedKey`, and `alertMsg`) so that when reason is empty you either translate a fallback key or append a meaningful fallback message rather than an empty suffix.
🤖 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/Transport.ts`:
- Around line 400-402: The hard-coded user-visible label "--- English ---" in
Transport.ts should be replaced with a translated string: call
translateText('english_section_label') (or similar key) instead of the literal,
and use that value when building alertMsg alongside
getEnglishText(connRefusedKey). Also add the corresponding English locale entry
for 'english_section_label' in the app's i18n resources so the label is
available for translateText().
---
Duplicate comments:
In `@src/client/Transport.ts`:
- Around line 383-390: Guard against an empty CloseEvent.reason before
constructing errorKey; if event.reason is falsy or only whitespace, avoid
building `worker_error.${event.reason}` and instead use a fallback key or
include a descriptive fallback string (e.g., "unknown" or the numeric close
code) when calling translateText. Update the code around the errorKey creation
and the logic that appends to alertMsg (referencing the variables `event`,
`errorKey`, `translateText`, `connRefusedKey`, and `alertMsg`) so that when
reason is empty you either translate a fallback key or append a meaningful
fallback message rather than an empty suffix.
🪄 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
Run ID: fa5cd2c7-9f28-4663-9a77-ebe8d59e9222
📒 Files selected for processing (2)
resources/lang/en.jsonsrc/client/Transport.ts
| const englishMsg = getEnglishText(errorKey); | ||
| if (englishMsg !== errorKey && !alertMsg.includes(englishMsg)) { | ||
| alertMsg += `\n\n--- English ---\n${getEnglishText(connRefusedKey)}: ${englishMsg}`; |
There was a problem hiding this comment.
Translate the English-section label too.
"--- English ---" is user-visible client text, so it should come from translateText() with a matching locale entry instead of being hard-coded here.
🌐 Minimal fix
- alertMsg += `\n\n--- English ---\n${getEnglishText(connRefusedKey)}: ${englishMsg}`;
+ alertMsg += `\n\n--- ${translateText("worker_error.english_label")} ---\n${getEnglishText(connRefusedKey)}: ${englishMsg}`; "worker_error": {
"account_banned": "Account Banned",
"cannot_join_game": "Cannot join game",
"connection_refused": "Connection refused",
+ "english_label": "English",
"forbidden": "Forbidden",As per coding guidelines "All user-visible text must go through translateText() function with corresponding entries in resources/lang/en.json".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const englishMsg = getEnglishText(errorKey); | |
| if (englishMsg !== errorKey && !alertMsg.includes(englishMsg)) { | |
| alertMsg += `\n\n--- English ---\n${getEnglishText(connRefusedKey)}: ${englishMsg}`; | |
| const englishMsg = getEnglishText(errorKey); | |
| if (englishMsg !== errorKey && !alertMsg.includes(englishMsg)) { | |
| alertMsg += `\n\n--- ${translateText("worker_error.english_label")} ---\n${getEnglishText(connRefusedKey)}: ${englishMsg}`; |
🤖 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/Transport.ts` around lines 400 - 402, The hard-coded user-visible
label "--- English ---" in Transport.ts should be replaced with a translated
string: call translateText('english_section_label') (or similar key) instead of
the literal, and use that value when building alertMsg alongside
getEnglishText(connRefusedKey). Also add the corresponding English locale entry
for 'english_section_label' in the app's i18n resources so the label is
available for translateText().
seems like the error message could get pretty huge if we do that? maybe better do do like error code 1,2,3,4,5,6,7,8, etc ? |
The error messages sent from the Worker on close with reason 1002 to be shown in the alert box, are not that big. No problem really. Error modal in ClientGameRunner is huge in comparison with the info in it, these are quite short. And if we really want them shorter in the future, we can always do that in a follow-up but again don't think that's needed. |
| `WebSocket closed. Code: ${event.code}, Reason: ${event.reason}`, | ||
| ); | ||
| if (event.code === 1002) { | ||
| const connRefusedKey = `worker_error.connection_refused`; |
There was a problem hiding this comment.
this is a bit messy. I'm thinking we do something like create a zod schema for websocket error and it's just something like:
const ErrorSchema = z.object({
translationKey: z.string()
args: z.record<z.string(),z.string()>
})
there's a 123 byte limit, so maybe we should send the error and then close:
import { z } from "zod";
import type { WebSocket } from "ws";
export const WSErrorSchema = z.object({
translationKey: z.string(),
args: z.record(z.string(), z.string()).optional(),
});
export type WSError = z.infer;
export function sendErrorAndClose(ws: WebSocket, error: WSError, code = 4000) {
if (ws.readyState === ws.OPEN) {
ws.send(JSON.stringify(error));
}
ws.close(code);
}
There was a problem hiding this comment.
Added it. Don't think it is much less 'messy'. Because we still need to test stuff, since we don't know if A) we recieved an actual translation key and not just a raw error string like "ClientJoinMessageSchema", "WS_ERR_UNEXPECTED_RSV_1" and in cosmetics.reason and B) if we did recieve a translation key, the translation may be missing from both the user's labguage JSON and en.json and translateText will send back the translation key as a result.
To clarify it a bit more, i could rename "translationKey" in WSErrorSchema and the const in Transport.ts to something like "translationKeyOrError" because it can contain both.
There was a problem hiding this comment.
yeah let's just not support args and then it's really simple like:
ws.close(1002, worker_error.game_not_found)
and then here we just do:
alert(translateText(ws.reason))
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/Worker.ts (1)
426-437:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass cosmetic validation errors with a fixed translation key, not the raw reason string.
The
cosmeticResult.reasoncontains raw error messages like"invalid pattern: ...","invalid color: ...", and"invalid flag: ...". These are not translation keys. Passing them directly astranslationKeymeans the client will try to findworker_error.invalid pattern: ...in the language files, which does not exist. The client falls back to showing the raw untranslated string—the opposite of your stated design.Use a fixed translation key and pass the raw reason through
args:- sendErrorAndClose(ws, { translationKey: cosmeticResult.reason }); + sendErrorAndClose(ws, { + translationKey: "forbidden_cosmetic", + args: { reason: cosmeticResult.reason }, + });Then add
"forbidden_cosmetic"to theworker_errorsection of your language files. The client already reads theargsfield, so it can display the raw reason separately if needed for debugging without trying to translate it.🤖 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/server/Worker.ts` around lines 426 - 437, The cosmetic validation currently sends cosmeticResult.reason as the translationKey (from privilegeRefresher.get().isAllowed) which are raw messages; instead, change the error sent via sendErrorAndClose in the Worker.ts block handling cosmeticResult.type === "forbidden" to use a fixed translation key like "worker_error.forbidden_cosmetic" (or "forbidden_cosmetic" depending on your keys) and include the raw cosmeticResult.reason in the error payload as an args property (e.g., args: { reason: cosmeticResult.reason } or args: [cosmeticResult.reason]) so the client can display the raw detail without treating it as a translation key; also add the "forbidden_cosmetic" entry to the worker_error section of your language files.
🧹 Nitpick comments (5)
src/server/Worker.ts (3)
305-313: ⚡ Quick winDouble error send before close — drop the legacy
ServerErrorMessagebranch.This path now sends two JSON messages back-to-back before closing: the legacy
{ type: "error", error }payload and then the new{ translationKey: "ClientJoinMessageSchema" }payload. The client will only act on one of them, and parsing both creates noise.Pick one. If the new
WSErrorflow is the path forward (it is, per the rest of this file), drop the old send.♻️ Suggested cleanup
- if (!parsed.success) { - const error = z.prettifyError(parsed.error); - log.warn("Error parsing client message", error); - ws.send( - JSON.stringify({ - type: "error", - error: error.toString(), - } satisfies ServerErrorMessage), - ); - sendErrorAndClose(ws, { translationKey: "ClientJoinMessageSchema" }); - return; - } + if (!parsed.success) { + const error = z.prettifyError(parsed.error); + log.warn("Error parsing client message", error); + sendErrorAndClose(ws, { + translationKey: "worker_error.invalid_client_message", + }); + return; + }🤖 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/server/Worker.ts` around lines 305 - 313, Remove the legacy ServerErrorMessage send so we don't emit two error messages: delete the ws.send(JSON.stringify({ type: "error", error: error.toString() } satisfies ServerErrorMessage)) call and rely solely on the new WSError flow via sendErrorAndClose(ws, { translationKey: "ClientJoinMessageSchema" }); — update any related types/imports if unused (ServerErrorMessage) and ensure no other code paths still emit both messages.
500-508: ⚡ Quick win
catchblock still uses the oldws.close(1011, ...)style.The whole point of this PR is to make every disconnect from the worker carry a translatable payload, but the generic
try/catchstill closes with a raw English reason and noWSError. Users hitting this path will see only the close-code path on the client.For consistency, route this through
sendErrorAndClosetoo:♻️ Suggested fix
- } catch (error) { - ws.close(1011, "Internal server error"); + } catch (error) { + sendErrorAndClose( + ws, + { translationKey: "internal_server_error" }, + 1011, + ); log.warn( `error handling websocket message for ${ipAnonymize(ip)}: ${error}`.substring( 0, 250, ), ); }🤖 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/server/Worker.ts` around lines 500 - 508, The catch block currently calls ws.close(1011, ...) with a raw string; replace that with a call to sendErrorAndClose(ws, new WSError("internal_server_error", { message: String(error) })) (or the project's WSError constructor/pattern) so the disconnect carries a translatable payload; locate the catch around the websocket message handler in Worker.ts (the block that references ws, ipAnonymize(ip), and log.warn) and invoke sendErrorAndClose there, then log the error via log.warn as before using ipAnonymize(ip) and the error details.
38-43: 💤 Low valueHelper is fine; consider not hard-coding
1002as the default.
1002means "protocol error" in the WebSocket spec, but most callers here are using it for application-level rejections (lobby full, banned, forbidden flag, etc.). It still works because the client just looks at the JSON body, but the close code is misleading to anyone reading network traces.A small idiomatic tweak: drop the default and let each caller pass the close code that fits, or use
1008("policy violation") for auth/permission denials and1011("server error") for the unexpected ones. Not blocking.Also worth noting:
ws.sendqueues the frame;ws.close()immediately after will still flush the queued payload before sending the close frame, so the ordering is safe — no callback needed.🤖 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/server/Worker.ts` around lines 38 - 43, The helper sendErrorAndClose currently hard-codes a default close code of 1002 (protocol error); remove that default so callers must supply an explicit close code and update all call sites to pass the most appropriate code (use 1008 for auth/permission/policy violations like banned/forbidden/lobby-full, 1011 for unexpected server errors). Locate sendErrorAndClose in Worker.ts and change the signature to require the code parameter, then audit every place that calls sendErrorAndClose and supply the correct numeric close code per the above guidance (no need to alter send/close ordering—queued send will flush before the close frame).src/core/Schemas.ts (1)
616-620: 💤 Low valueSchema looks clean — small note on
argsvalue type.The shape is simple and idiomatic Zod. One small thing to consider:
argsonly allowsstringvalues, so any numeric/boolean placeholders in i18n strings must be stringified by callers. If you ever expect counts/durations, you may want to widen this toz.union([z.string(), z.number(), z.boolean()])later. Not blocking.🤖 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/core/Schemas.ts` around lines 616 - 620, The WSErrorSchema's args field currently restricts values to strings only, which forces callers to stringify numbers/booleans; update WSErrorSchema (the args property) to accept non-string primitives (e.g., widen the type to a union of string | number | boolean or similar) so placeholders like counts/durations can be passed directly, and ensure the corresponding WSError type (z.infer<typeof WSErrorSchema>) reflects the widened value types.src/server/GameServer.ts (1)
31-31: ⚡ Quick winMove
sendErrorAndCloseto its own small module.
Worker.tsis the worker bootstrap (it starts servers, polls matchmaking, owns WSS state). Pulling a tiny pure helper out of it fromGameServer.tscouples two modules that otherwise have no reason to know about each other, and it means anyone importing the helper dragsWorker.ts's top-level setup into their build graph.Suggest extracting it into something like
src/server/WSError.ts(orsrc/server/wsClose.ts) and importing it from there in bothWorker.tsandGameServer.ts.🤖 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/server/GameServer.ts` at line 31, The import of sendErrorAndClose from Worker.ts couples GameServer.ts to Worker.ts's heavy bootstrap; extract sendErrorAndClose into a tiny dedicated module (e.g. src/server/WSError.ts or src/server/wsClose.ts) that exports the function, move the implementation there ensuring it has no Worker-side effects, then update both GameServer.ts and Worker.ts to import sendErrorAndClose from the new module instead of "./Worker". Keep the new module minimal and pure so importing it does not pull Worker.ts's top-level setup into the build graph.
🤖 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/server/Worker.ts`:
- Line 311: The call to sendErrorAndClose is passing a placeholder
translationKey "ClientJoinMessageSchema" which won't match worker_error keys in
resources/lang/en.json; change the key to a snake_case worker_error entry like
"invalid_client_message" (or "bad_request") in the sendErrorAndClose call and
add the corresponding message under the worker_error object in
resources/lang/en.json so the user sees a proper localized message instead of
the raw schema name.
---
Outside diff comments:
In `@src/server/Worker.ts`:
- Around line 426-437: The cosmetic validation currently sends
cosmeticResult.reason as the translationKey (from
privilegeRefresher.get().isAllowed) which are raw messages; instead, change the
error sent via sendErrorAndClose in the Worker.ts block handling
cosmeticResult.type === "forbidden" to use a fixed translation key like
"worker_error.forbidden_cosmetic" (or "forbidden_cosmetic" depending on your
keys) and include the raw cosmeticResult.reason in the error payload as an args
property (e.g., args: { reason: cosmeticResult.reason } or args:
[cosmeticResult.reason]) so the client can display the raw detail without
treating it as a translation key; also add the "forbidden_cosmetic" entry to the
worker_error section of your language files.
---
Nitpick comments:
In `@src/core/Schemas.ts`:
- Around line 616-620: The WSErrorSchema's args field currently restricts values
to strings only, which forces callers to stringify numbers/booleans; update
WSErrorSchema (the args property) to accept non-string primitives (e.g., widen
the type to a union of string | number | boolean or similar) so placeholders
like counts/durations can be passed directly, and ensure the corresponding
WSError type (z.infer<typeof WSErrorSchema>) reflects the widened value types.
In `@src/server/GameServer.ts`:
- Line 31: The import of sendErrorAndClose from Worker.ts couples GameServer.ts
to Worker.ts's heavy bootstrap; extract sendErrorAndClose into a tiny dedicated
module (e.g. src/server/WSError.ts or src/server/wsClose.ts) that exports the
function, move the implementation there ensuring it has no Worker-side effects,
then update both GameServer.ts and Worker.ts to import sendErrorAndClose from
the new module instead of "./Worker". Keep the new module minimal and pure so
importing it does not pull Worker.ts's top-level setup into the build graph.
In `@src/server/Worker.ts`:
- Around line 305-313: Remove the legacy ServerErrorMessage send so we don't
emit two error messages: delete the ws.send(JSON.stringify({ type: "error",
error: error.toString() } satisfies ServerErrorMessage)) call and rely solely on
the new WSError flow via sendErrorAndClose(ws, { translationKey:
"ClientJoinMessageSchema" }); — update any related types/imports if unused
(ServerErrorMessage) and ensure no other code paths still emit both messages.
- Around line 500-508: The catch block currently calls ws.close(1011, ...) with
a raw string; replace that with a call to sendErrorAndClose(ws, new
WSError("internal_server_error", { message: String(error) })) (or the project's
WSError constructor/pattern) so the disconnect carries a translatable payload;
locate the catch around the websocket message handler in Worker.ts (the block
that references ws, ipAnonymize(ip), and log.warn) and invoke sendErrorAndClose
there, then log the error via log.warn as before using ipAnonymize(ip) and the
error details.
- Around line 38-43: The helper sendErrorAndClose currently hard-codes a default
close code of 1002 (protocol error); remove that default so callers must supply
an explicit close code and update all call sites to pass the most appropriate
code (use 1008 for auth/permission/policy violations like
banned/forbidden/lobby-full, 1011 for unexpected server errors). Locate
sendErrorAndClose in Worker.ts and change the signature to require the code
parameter, then audit every place that calls sendErrorAndClose and supply the
correct numeric close code per the above guidance (no need to alter send/close
ordering—queued send will flush before the close frame).
🪄 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
Run ID: bd66dd13-3716-4e40-8a59-a8cdc9ffe20b
📒 Files selected for processing (4)
src/client/Transport.tssrc/core/Schemas.tssrc/server/GameServer.tssrc/server/Worker.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/Transport.ts
3 participants
Description:
For v31 so we and players can profit from it as soon as possible.
Explain ways to fix turnstile token invalid/rejected, instead of only displaying raw error to user.
I think this is also recommended by Cloudflare and for good reason. Most users are helped by these instructions and now they don't have to go ask support via email or Discord.
Addition later on:
Translate most 1002 errors that are displayed in this alert box. Always starts with translation for "connection refused: " followed by translated error. Except for "ClientMessageSchema", "WS_ERR_UNEXPECTED_RSV_1" raw error.reason strings. Cosmetic.reason string also isn't translated, doesn't seem like going another level deeper really helps.
Add translated tips if turnstile is invalid. Append English error for screenshots so devs understand what error user gets.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
tryout33