feat: surface unprocessable state deltas as fatal client errors - #6827
feat: surface unprocessable state deltas as fatal client errors#6827FarhanAliRaza wants to merge 5 commits into
Conversation
When the backend sends a delta with a substate the frontend has no dispatch function for (mismatched frontend/backend state definitions), the frontend now: - validates the entire delta before dispatching anything, so a bad substate no longer partially applies an update or silently drops queued events, - logs an actionable error to the browser console, - reports the error to the backend via a new client_error socket event so it shows up in the terminal where devs look first, - treats the mismatch as fatal per reflex-dev#6019: no further events are sent until the frontend is rebuilt/reloaded, instead of erroring again on every interaction. Unexpected errors while applying a delta are likewise reported to the backend instead of vanishing as unhandled rejections. The backend on_client_error handler validates the payload shape, sanitizes and truncates client-supplied strings before logging, and only logs at error level for sockets with a linked token. Error type strings are shared via constants.ClientErrorType, and emit_update gained debug logging of outgoing substates (guarded by is_debug so the hot path is unaffected). Fixes reflex-dev#6019
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR makes unprocessable frontend state deltas fatal and reports them to the backend.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/.templates/web/utils/state.js | Adds pre-dispatch delta validation, fatal mismatch handling, one-time reload recovery, and backend error reporting. |
| reflex/app.py | Adds sanitized client-error routing with per-session and process-wide rate limits. |
| packages/reflex-base/src/reflex_base/utils/format.py | Adds bounded sanitization for untrusted values sent to backend logs. |
| tests/units/test_app.py | Covers client-error handling, sanitization, malformed senders, constants, and both rate limits. |
| tests/integration/tests_playwright/test_client_error.py | Exercises backend mismatch reporting and one-time frontend reload behavior end to end. |
Reviews (5): Last reviewed commit: "fix: reload once on frontend/backend sta..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfd1341892
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
- Rate-limit error-level client_error logging to 5 entries per SID (cleared on disconnect) so a client that links an arbitrary token cannot flood backend logs. - Escape rich markup in sanitized client values; unescaped closing tags raised MarkupError and styling tags could inject into terminal logs. - Keep sanitized values within max_length including the truncation suffix. - Clear the event queue on fatal state mismatch; callers drain the queue in while-loops that would otherwise spin forever. - Await queueEvents inside the event handler try block so failures are reported via client_error instead of unhandled rejections; guard error.message for non-Error throws. - Add news fragments for the changelog check.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Per-SID budgets reset when a new socket connects, so scripted reconnect loops could still flood backend logs. Add a process-wide time-window cap (20 entries per 60s) on top of the per-SID limit; later windows log again, so long-lived sessions are not silenced forever.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Fix all with cubic | Re-trigger cubic
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
if we're introducing a new client error message type on the transport, then we should wire up the existing frontend error handler to this mechanism.
- Wire on_client_error into app.frontend_exception_handler so custom handlers (e.g. error trackers) receive client-reported errors. - Reword the frontend mismatch message to suggest refreshing the page first, per review. - Remove the per-update substate debug log (too spammy for --loglevel debug). - Warn once per window when the client_error rate limit trips so suppression is never silent.
|
Wired the client_error mechanism into the existing frontend error handler in ba8beb3: |
A stale frontend build is the usual cause of a delta the frontend cannot dispatch, and a reload picks up the matching one. The reload is recorded in sessionStorage so a mismatch that survives it (e.g. api_url pointing at a different app) leaves the page up with the error reported rather than looping. Also from review: - Move _sanitize_client_log_value off EventNamespace into format.sanitize_client_log_value, slicing to max_length before the per-character scan so an oversized value costs no more than a bounded one. - Sanitize only after the SID and rate-limit checks, so reports that get dropped do not pay for it. - Escape rich markup in FrontendEventExceptionState, where a JS message containing square brackets could style backend logs or raise MarkupError. - Validate the delta in a single pass that allocates only on a miss, and stop awaiting queueEvents inside the event handler so a delta applies in full before a later update can interleave. - Fold tests/units/test_client_error.py into test_app.py and test_format.py, and cover the report and the reload in Playwright.
There was a problem hiding this comment.
3 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-base/src/reflex_base/utils/format.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/utils/format.py:631">
P2: `sanitize_client_log_value` can return strings longer than `max_length` for very small limits, because the truncation branch always appends the full `... (truncated)` suffix after a negative-index slice. Handling the `max_length <= len(suffix)` case separately keeps the output hard-bounded.
(Based on your team's feedback about respecting log-value max length.) .</violation>
</file>
<file name="tests/integration/tests_playwright/test_client_error.py">
<violation number="1" location="tests/integration/tests_playwright/test_client_error.py:147">
P3: The final assertion in test_unprocessable_delta_reloads_once is effectively a tautology: after the reload the counter renders its default "0", so `not_to_have_text("")` passes whether or not the socket works and regardless of the bump click. It therefore doesn't back up the "page is usable again" claim. Consider asserting a value the click actually changes, e.g. `to_have_text("1")` as in the first test, so the post-reload reload-once + functional check is meaningful.</violation>
</file>
<file name="packages/reflex-base/src/reflex_base/.templates/web/utils/state.js">
<violation number="1" location="packages/reflex-base/src/reflex_base/.templates/web/utils/state.js:739">
P1: Event-only socket updates can now fail in the frontend handler because the new validation loop iterates `update.delta` unconditionally. Using a null-safe loop input keeps `update.events` processing working when `delta` is omitted.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| // does not result in a partially applied state update. Walk the delta once | ||
| // and only allocate when a substate is actually missing. | ||
| let missing_substates; | ||
| for (const substate in update.delta) { |
There was a problem hiding this comment.
P1: Event-only socket updates can now fail in the frontend handler because the new validation loop iterates update.delta unconditionally. Using a null-safe loop input keeps update.events processing working when delta is omitted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/.templates/web/utils/state.js, line 739:
<comment>Event-only socket updates can now fail in the frontend handler because the new validation loop iterates `update.delta` unconditionally. Using a null-safe loop input keeps `update.events` processing working when `delta` is omitted.</comment>
<file context>
@@ -712,18 +716,32 @@ export const connect = async (
+ // does not result in a partially applied state update. Walk the delta once
+ // and only allocate when a substate is actually missing.
+ let missing_substates;
+ for (const substate in update.delta) {
+ if (typeof dispatch[substate] !== "function") {
+ (missing_substates ??= []).push(substate);
</file context>
| for (const substate in update.delta) { | |
| for (const substate in update.delta ?? {}) { |
| truncated = True | ||
| if truncated: | ||
| suffix = "... (truncated)" | ||
| text = text[: max_length - len(suffix)] + suffix |
There was a problem hiding this comment.
P2: sanitize_client_log_value can return strings longer than max_length for very small limits, because the truncation branch always appends the full ... (truncated) suffix after a negative-index slice. Handling the max_length <= len(suffix) case separately keeps the output hard-bounded.
(Based on your team's feedback about respecting log-value max length.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/utils/format.py, line 631:
<comment>`sanitize_client_log_value` can return strings longer than `max_length` for very small limits, because the truncation branch always appends the full `... (truncated)` suffix after a negative-index slice. Handling the `max_length <= len(suffix)` case separately keeps the output hard-bounded.
(Based on your team's feedback about respecting log-value max length.) .</comment>
<file context>
@@ -601,6 +603,35 @@ def format_query_params(router_data: dict[str, Any]) -> dict[str, str]:
+ truncated = True
+ if truncated:
+ suffix = "... (truncated)"
+ text = text[: max_length - len(suffix)] + suffix
+ return text
+
</file context>
| text = text[: max_length - len(suffix)] + suffix | |
| if max_length <= len(suffix): | |
| text = suffix[:max_length] | |
| else: | |
| text = text[: max_length - len(suffix)] + suffix |
| # The page is usable again after the reload. | ||
| expect(page.locator("#token")).not_to_have_value("") | ||
| page.click("#bump-btn") | ||
| expect(page.locator("#counter")).not_to_have_text("") |
There was a problem hiding this comment.
P3: The final assertion in test_unprocessable_delta_reloads_once is effectively a tautology: after the reload the counter renders its default "0", so not_to_have_text("") passes whether or not the socket works and regardless of the bump click. It therefore doesn't back up the "page is usable again" claim. Consider asserting a value the click actually changes, e.g. to_have_text("1") as in the first test, so the post-reload reload-once + functional check is meaningful.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/integration/tests_playwright/test_client_error.py, line 147:
<comment>The final assertion in test_unprocessable_delta_reloads_once is effectively a tautology: after the reload the counter renders its default "0", so `not_to_have_text("")` passes whether or not the socket works and regardless of the bump click. It therefore doesn't back up the "page is usable again" claim. Consider asserting a value the click actually changes, e.g. `to_have_text("1")` as in the first test, so the post-reload reload-once + functional check is meaningful.</comment>
<file context>
@@ -0,0 +1,147 @@
+ # The page is usable again after the reload.
+ expect(page.locator("#token")).not_to_have_value("")
+ page.click("#bump-btn")
+ expect(page.locator("#counter")).not_to_have_text("")
</file context>
| expect(page.locator("#counter")).not_to_have_text("") | |
| expect(page.locator("#counter")).to_have_text("1") |
2 participants
Summary
When the backend sends a delta the frontend cannot process (no dispatch function registered for a substate), the app currently throws
dispatch[substate] is not a functionin the browser console and otherwise looks silently broken. This PR makes that failure loud, actionable, and visible in the backend terminal — where Python devs look first.Fixes #6019. Supersedes #6128, rebuilt on top of the
packages/reflex-baselayout with the review findings from that PR addressed.Frontend (
reflex_base/.templates/web/utils/state.js)update.events.client_errorsocket event.backend_state_mismatchflag stops all further event sending and drops incoming updates, so the error is reported once instead of on every interaction. A page reload (e.g. after rebuilding the frontend) resets it.Backend (
reflex/app.py)EventNamespace.on_client_errorhandler logs frontend-reported errors in the terminal with remediation steps (rebuild frontend / checkapi_url).emit_updategained debug logging of outgoing substates, guarded byconsole.is_debug()so the hot path doesn't pay for message construction.Shared constants
SocketEvent.CLIENT_ERRORand a newClientErrorTypenamespace inreflex_base.constants.eventkeep the error-type strings in one place, matched by theERROR_TYPE_*constants instate.js.Testing
tests/units/test_client_error.pycovers both error branches, malformed payloads (which previously raisedAttributeError), unknown-sid gating, and sanitization/truncation.tests/units/test_app.pyandtests/units/utils/test_token_manager.pypass alongside (164 passed).