diff --git a/news/6827.feature.md b/news/6827.feature.md new file mode 100644 index 00000000000..7643a7a3ddf --- /dev/null +++ b/news/6827.feature.md @@ -0,0 +1 @@ +Report state deltas the frontend cannot process back to the backend via a new `client_error` socket event, logging an actionable error in the terminal instead of failing silently. A frontend/backend state mismatch is fatal for the session: further events stop until the page is reloaded after the frontend is rebuilt or `api_url` is corrected. diff --git a/packages/reflex-base/news/6827.feature.md b/packages/reflex-base/news/6827.feature.md new file mode 100644 index 00000000000..b8b3c33e55d --- /dev/null +++ b/packages/reflex-base/news/6827.feature.md @@ -0,0 +1 @@ +Validate incoming state deltas in the frontend before dispatching and report unprocessable updates to the backend via a new `client_error` socket event instead of failing silently in the browser console. Values reported by a client are escaped and bounded before reaching the backend logs. diff --git a/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js b/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js index 05af3acc362..8ba6d00509c 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js @@ -24,6 +24,13 @@ import { uploadFiles } from "$/utils/helpers/upload"; // Endpoint URLs. const EVENTURL = env.EVENT; +// Socket event names (must match reflex_base/constants/event.py SocketEvent) +const CLIENT_ERROR_EVENT = "client_error"; + +// Client error types (must match reflex_base/constants/event.py ClientErrorType) +const ERROR_TYPE_DISPATCH_MISSING = "dispatch_function_missing"; +const ERROR_TYPE_STATE_UPDATE = "state_update_processing_error"; + // These hostnames indicate that the backend and frontend are reachable via the same domain. const SAME_DOMAIN_HOSTNAMES = ["localhost", "0.0.0.0", "::", "0:0:0:0:0:0:0:0"]; @@ -39,6 +46,10 @@ const cookies = new Cookies(); // Dictionary holding component references. export const refs = {}; +// Set when the backend sends a delta the frontend cannot process. A mismatch +// between frontend and backend state definitions is fatal (#6019): no further +// events are sent until the frontend is rebuilt/reloaded. +let backend_state_mismatch = false; // Array holding pending events to be processed. const event_queue = []; @@ -519,6 +530,14 @@ export const processEvent = async (socket, navigate, params) => { return; } + // A backend/frontend state mismatch is fatal; do not send further events. + // Drop pending events too: callers drain the queue in while-loops that + // would otherwise spin forever on an early return. + if (backend_state_mismatch) { + event_queue.length = 0; + return; + } + // Only proceed if we're not already processing an event. if (event_queue.length === 0) { return; @@ -693,24 +712,76 @@ export const connect = async ( } }); + // Report a failure to process a state update to the backend, so it surfaces + // in the terminal logs instead of only in the browser console. + const reportStateUpdateError = (error) => { + console.error("Error processing state update:", error); + socket.current?.emit(CLIENT_ERROR_EVENT, { + message: error?.message || String(error), + error_type: ERROR_TYPE_STATE_UPDATE, + }); + }; + // On each received message, queue the updates and events. - socket.current.on("event", async (update) => { - if (update.delta && Object.keys(update.delta).length > 0) { - for (const substate in update.delta) { - dispatch[substate](update.delta[substate]); - // handle events waiting for `is_hydrated` - if ( - substate === state_name && - update.delta[substate]?.is_hydrated_rx_state_ - ) { - queueEvents(on_hydrated_queue, socket, false, navigate, params); - on_hydrated_queue.length = 0; - } + socket.current.on("event", (update) => { + if (backend_state_mismatch) { + // A fatal state mismatch was already detected; drop further updates. + return; + } + // Validate the whole delta before dispatching anything, so a bad substate + // 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); } - applyClientStorageDelta(client_storage, update.delta); } - if (update.events && update.events.length > 0) { - queueEvents(update.events, socket, false, navigate, params); + if (missing_substates !== undefined) { + const errorMsg = `Cannot process state update: no dispatch function for substate(s) "${missing_substates.join( + '", "', + )}". Try refreshing the page or clearing your browser cache. This error usually indicates a mismatch between frontend and backend state definitions. If you are the developer of this app, rebuild the frontend and check that api_url is correct.`; + console.error(errorMsg); + // Surface the error in the backend terminal logs. + socket.current.emit(CLIENT_ERROR_EVENT, { + message: errorMsg, + substate: missing_substates.join(", "), + error_type: ERROR_TYPE_DISPATCH_MISSING, + }); + backend_state_mismatch = true; + return; + } + try { + if (update.delta) { + for (const substate in update.delta) { + dispatch[substate](update.delta[substate]); + // handle events waiting for `is_hydrated` + if ( + substate === state_name && + update.delta[substate]?.is_hydrated_rx_state_ + ) { + // Deliberately not awaited: the rest of the delta and the client + // storage below must be applied before this handler yields, or a + // later update can interleave and apply its delta first. + queueEvents( + on_hydrated_queue, + socket, + false, + navigate, + params, + ).catch(reportStateUpdateError); + on_hydrated_queue.length = 0; + } + } + applyClientStorageDelta(client_storage, update.delta); + } + if (update.events && update.events.length > 0) { + queueEvents(update.events, socket, false, navigate, params).catch( + reportStateUpdateError, + ); + } + } catch (error) { + reportStateUpdateError(error); } }); socket.current.on("new_token", async (new_token) => { diff --git a/packages/reflex-base/src/reflex_base/constants/__init__.py b/packages/reflex-base/src/reflex_base/constants/__init__.py index 714cf0faa84..f83bcfd1e25 100644 --- a/packages/reflex-base/src/reflex_base/constants/__init__.py +++ b/packages/reflex-base/src/reflex_base/constants/__init__.py @@ -53,7 +53,7 @@ UvLock, ) from .custom_components import CustomComponents -from .event import Endpoint, EventTriggers, SocketEvent +from .event import ClientErrorType, Endpoint, EventTriggers, SocketEvent from .installer import Bun, Node, PackageJson from .route import ( ROUTE_NOT_FOUND, @@ -92,6 +92,7 @@ "SYSTEM_COLOR_MODE", "AgentsMd", "Bun", + "ClientErrorType", "ColorMode", "CompileContext", "CompileVars", diff --git a/packages/reflex-base/src/reflex_base/constants/event.py b/packages/reflex-base/src/reflex_base/constants/event.py index 515b34574e9..2fa59c5bb97 100644 --- a/packages/reflex-base/src/reflex_base/constants/event.py +++ b/packages/reflex-base/src/reflex_base/constants/event.py @@ -49,6 +49,7 @@ class SocketEvent(SimpleNamespace): PING = "ping" EVENT = "event" + CLIENT_ERROR = "client_error" def __str__(self) -> str: """Get the string representation of the event name. @@ -59,6 +60,16 @@ def __str__(self) -> str: return str(self.value) +class ClientErrorType(SimpleNamespace): + """Error types reported by the frontend via the client_error socket event. + + Must match the ERROR_TYPE_* constants in .templates/web/utils/state.js. + """ + + DISPATCH_MISSING = "dispatch_function_missing" + STATE_UPDATE = "state_update_processing_error" + + class EventTriggers(SimpleNamespace): """All trigger names used in Reflex.""" diff --git a/packages/reflex-base/src/reflex_base/utils/format.py b/packages/reflex-base/src/reflex_base/utils/format.py index 35a2bc3e30a..d7173f7b598 100644 --- a/packages/reflex-base/src/reflex_base/utils/format.py +++ b/packages/reflex-base/src/reflex_base/utils/format.py @@ -9,6 +9,8 @@ from functools import lru_cache from typing import TYPE_CHECKING, Any +from rich.markup import escape as escape_markup + from reflex_base import constants from reflex_base.utils import exceptions @@ -601,6 +603,35 @@ def format_query_params(router_data: dict[str, Any]) -> dict[str, str]: return {k.replace("-", "_"): v for k, v in params.items()} +def sanitize_client_log_value(value: Any, max_length: int = 500) -> str: + """Make a client-supplied value safe to write to backend logs. + + Args: + value: The client-supplied value. + max_length: Maximum length of the returned string. + + Returns: + The value as a printable, length-bounded string with control characters + (newlines, ANSI escapes) replaced by spaces and rich markup escaped, so + a client cannot forge log lines, style backend output, or raise + ``MarkupError`` when the value is printed through the console helpers. + """ + text = value if isinstance(value, str) else str(value) + # Slice before the per-character walk: clients can send arbitrarily long + # values, and everything past max_length is discarded anyway. + truncated = len(text) > max_length + text = escape_markup( + "".join(char if char.isprintable() else " " for char in text[:max_length]) + ) + if len(text) > max_length: + # Escaping markup can push a value that just fit over the limit. + truncated = True + if truncated: + suffix = "... (truncated)" + text = text[: max_length - len(suffix)] + suffix + return text + + def format_state_name(state_name: str) -> str: """Format a state name, replacing dots with double underscore. diff --git a/reflex/app.py b/reflex/app.py index 509b6dcdbfc..2affd0b463c 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -1908,6 +1908,16 @@ class EventNamespace(AsyncNamespace): # The application object. app: App + # Maximum error-level log entries a single session may produce via the + # client_error event before further reports from it are dropped. + _MAX_CLIENT_ERRORS_PER_SID = 5 + + # Process-wide bound on error-level client_error log entries per time + # window; per-SID budgets alone reset on reconnect, so scripted + # reconnects could otherwise flood the logs. + _CLIENT_ERROR_WINDOW_SECONDS = 60.0 + _MAX_CLIENT_ERRORS_PER_WINDOW = 20 + def __init__(self, namespace: str, app: App): """Initialize the event namespace. @@ -1921,6 +1931,13 @@ def __init__(self, namespace: str, app: App): # Use TokenManager for distributed duplicate tab prevention self._token_manager = TokenManager.create() + # Number of client_error reports logged per SID, for rate limiting. + self._client_error_counts: dict[str, int] = {} + + # Start time and count of the current process-wide client_error window. + self._client_error_window_start = 0.0 + self._client_error_window_count = 0 + @property def token_to_sid(self) -> Mapping[str, str]: """Get token to SID mapping for backward compatibility. @@ -1975,6 +1992,7 @@ def on_disconnect(self, sid: str) -> asyncio.Task | None: Returns: An asyncio Task for cleaning up the token, or None. """ + self._client_error_counts.pop(sid, None) # Get token before cleaning up disconnect_token = self.sid_to_token.get(sid) if disconnect_token: @@ -2119,6 +2137,83 @@ async def on_ping(self, sid: str): # Emit the test event. await self.emit(str(constants.SocketEvent.PING), "pong", to=sid) + async def on_client_error(self, sid: str, data: Any): + """Handle errors reported by the frontend. + + This is a dedicated socket event rather than a state event + (``FrontendEventExceptionState.handle_frontend_exception``) because a + state event is addressed by a handler name the frontend derives from + its own state definitions. When those definitions are what disagree + with the backend -- the case this handler exists to report -- the name + may not resolve and the report is lost. A fixed socket event name + cannot drift, and it still gets through after the frontend has stopped + sending events on detecting the mismatch. + + Reports are routed through the app's ``frontend_exception_handler``, + so frontend errors (especially state update processing errors) are + visible in backend logs and reach custom exception handlers. + + Args: + sid: The Socket.IO session id. + data: The error data from the client. + """ + if not isinstance(data, dict): + console.debug(f"Ignoring malformed client_error payload from SID {sid}.") + return + + # Check the sender and the rate limits before sanitizing: sanitizing is + # linear in the size of the client-supplied values, and reports that are + # dropped here must not cost more than the check itself. + if sid not in self.sid_to_token: + # Sockets without a linked token are not known clients; don't let + # them write error-level entries into the backend logs. + console.debug(f"Ignoring client_error report from unknown SID {sid}.") + return + + # Rate limit per session so a client cannot flood the backend logs. + error_count = self._client_error_counts.get(sid, 0) + if error_count >= self._MAX_CLIENT_ERRORS_PER_SID: + return + + # Also bound total entries per time window: per-SID budgets reset on + # reconnect, so they alone do not stop scripted reconnect loops. + now = time.monotonic() + if now - self._client_error_window_start > self._CLIENT_ERROR_WINDOW_SECONDS: + self._client_error_window_start = now + self._client_error_window_count = 0 + if self._client_error_window_count >= self._MAX_CLIENT_ERRORS_PER_WINDOW: + if self._client_error_window_count == self._MAX_CLIENT_ERRORS_PER_WINDOW: + # Warn once per window so suppression is visible in the logs + # and a flooding client cannot silently starve reports from + # other sessions. + self._client_error_window_count += 1 + console.warn( + f"Received more than {self._MAX_CLIENT_ERRORS_PER_WINDOW} " + f"client_error reports in {self._CLIENT_ERROR_WINDOW_SECONDS:.0f}s; " + "suppressing further reports for this window." + ) + return + self._client_error_window_count += 1 + self._client_error_counts[sid] = error_count + 1 + + error_type = format.sanitize_client_log_value(data.get("error_type", "unknown")) + if error_type == constants.ClientErrorType.DISPATCH_MISSING: + substate = format.sanitize_client_log_value(data.get("substate", "")) + report = ( + f"[SID: {sid}] State update failed: " + f"no dispatch function for substate(s) '{substate}'. " + "This indicates a frontend/backend state mismatch. " + "Rebuild the frontend or check that api_url points to the matching backend." + ) + else: + message = format.sanitize_client_log_value( + data.get("message", "No error message provided") + ) + report = f"[SID: {sid}] {error_type}: {message}" + # Route through the app's frontend exception handler so custom + # handlers (e.g. error trackers) receive client errors too. + self.app.frontend_exception_handler(Exception(report)) + async def link_token_to_sid(self, sid: str, token: str): """Link a token to a session id. diff --git a/reflex/constants/__init__.py b/reflex/constants/__init__.py index b37780a3acb..cc708858292 100644 --- a/reflex/constants/__init__.py +++ b/reflex/constants/__init__.py @@ -46,7 +46,7 @@ UvLock, ) from .custom_components import CustomComponents -from .event import Endpoint, EventTriggers, SocketEvent +from .event import ClientErrorType, Endpoint, EventTriggers, SocketEvent from .installer import Bun, Node, PackageJson from .route import ( ROUTE_NOT_FOUND, @@ -81,6 +81,7 @@ "SESSION_STORAGE", "SETTER_PREFIX", "Bun", + "ClientErrorType", "ColorMode", "CompileContext", "CompileVars", diff --git a/reflex/state.py b/reflex/state.py index f372d2db903..2fa3b479deb 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -2417,8 +2417,13 @@ def handle_frontend_exception( "window.location.reload();" "}" ) + # Escape rich markup so a JS error message containing square brackets + # (e.g. "x[/bold]y is not a function") cannot style backend logs or + # raise MarkupError when printed through the console helpers. The text + # is not otherwise sanitized: stack traces are multi-line by nature and + # truncating them would lose the information this handler exists for. prerequisites.get_and_validate_app().app.frontend_exception_handler( - Exception(info) + Exception(escape(info)) ) @@ -2605,7 +2610,13 @@ def create(cls, *children, **props) -> Component: frozen=True, ) class StateUpdate: - """A state update sent to the frontend.""" + """A state update sent to the frontend. + + Each substate key in the delta must have a dispatch function registered in + the frontend; otherwise the frontend reports a fatal ``client_error`` back + to the backend (see ``EventNamespace.on_client_error``), since this + indicates mismatched frontend and backend state definitions. + """ # The state delta. delta: DeltaMapping = dataclasses.field(default_factory=dict) diff --git a/tests/integration/tests_playwright/test_client_error.py b/tests/integration/tests_playwright/test_client_error.py new file mode 100644 index 00000000000..713b8d33dbf --- /dev/null +++ b/tests/integration/tests_playwright/test_client_error.py @@ -0,0 +1,164 @@ +"""Integration tests for reporting state deltas the frontend cannot process. + +A delta whose substate has no dispatch function in the compiled frontend means +the frontend and backend disagree about the state tree. The frontend reports it +back over the ``client_error`` socket event so the failure is visible in the +backend logs instead of being dropped silently, then stops sending events — +the mismatch is fatal until the developer rebuilds the frontend or fixes +``api_url``. +""" + +from __future__ import annotations + +from collections.abc import Generator + +import pytest +from playwright.sync_api import Page, expect + +from reflex.testing import AppHarness + +# Substate name with no dispatch function in the compiled frontend. +GHOST_SUBSTATE = "reflex___state____state____ghost_state" + + +def ClientErrorApp(): + """App that can emit a state delta the frontend cannot process.""" + import reflex as rx + from reflex.state import StateUpdate + + ghost_substate = "reflex___state____state____ghost_state" + + class ClientErrorState(rx.State): + counter: int = 0 + + @rx.event + def bump(self): + self.counter += 1 + + @rx.event + async def send_unprocessable_delta(self): + assert app.event_namespace is not None + await app.event_namespace.emit_update( + StateUpdate(delta={ghost_substate: {"value": 1}}), + self.router.session.client_token, + ) + + @rx.page("/") + def index(): + return rx.box( + rx.text(ClientErrorState.counter, id="counter"), + rx.input( + value=ClientErrorState.router.session.client_token, + read_only=True, + id="token", + ), + rx.button("bump", on_click=ClientErrorState.bump, id="bump-btn"), + rx.button( + "break", + on_click=ClientErrorState.send_unprocessable_delta, + id="break-btn", + ), + ) + + app = rx.App() + + +@pytest.fixture(scope="module") +def client_error_app( + tmp_path_factory: pytest.TempPathFactory, +) -> Generator[AppHarness, None, None]: + """Start the ClientErrorApp. + + Args: + tmp_path_factory: pytest fixture for creating temporary directories. + + Yields: + Running AppHarness instance. + """ + with AppHarness.create( + root=tmp_path_factory.mktemp("client_error_app"), + app_source=ClientErrorApp, + ) as harness: + assert harness.app_instance is not None, "app is not running" + yield harness + + +def test_unprocessable_delta_is_reported_to_backend( + client_error_app: AppHarness, page: Page, monkeypatch: pytest.MonkeyPatch +): + """An unprocessable delta reaches the backend's frontend exception handler. + + Args: + client_error_app: Running AppHarness instance. + page: Playwright page fixture. + monkeypatch: pytest fixture for patching the exception handler. + """ + assert client_error_app.frontend_url is not None + assert client_error_app.app_instance is not None + page.goto(client_error_app.frontend_url) + expect(page.locator("#token")).not_to_have_value("") + + # The socket works before the mismatch. + page.click("#bump-btn") + expect(page.locator("#counter")).to_have_text("1") + + reports: list[str] = [] + monkeypatch.setattr( + client_error_app.app_instance, + "frontend_exception_handler", + lambda exc: reports.append(str(exc)), + ) + + page.click("#break-btn") + + assert AppHarness._poll_for(lambda: reports), ( + "backend was not told about the unprocessable delta" + ) + report = reports[0] + assert GHOST_SUBSTATE in report + assert "no dispatch function" in report + assert "rebuild" in report.lower() + + +def test_unprocessable_delta_is_fatal_without_reload( + client_error_app: AppHarness, page: Page, monkeypatch: pytest.MonkeyPatch +): + """A mismatch is reported exactly once and does not reload the page. + + Recovery (rebuild or api_url fix) is the developer's decision, so the + frontend must not reload on its own or keep re-reporting. + + Args: + client_error_app: Running AppHarness instance. + page: Playwright page fixture. + monkeypatch: pytest fixture for patching the exception handler. + """ + assert client_error_app.frontend_url is not None + assert client_error_app.app_instance is not None + page.goto(client_error_app.frontend_url) + expect(page.locator("#token")).not_to_have_value("") + + reports: list[str] = [] + monkeypatch.setattr( + client_error_app.app_instance, + "frontend_exception_handler", + lambda exc: reports.append(str(exc)), + ) + + page.click("#break-btn") + assert AppHarness._poll_for(lambda: reports), ( + "backend was not told about the unprocessable delta" + ) + + # No automatic reload: the page's original navigation entry is still live. + assert ( + page.evaluate("() => performance.getEntriesByType('navigation')[0].type") + == "navigate" + ) + + # The mismatch is fatal: further clicks send no events and add no reports. + page.click("#break-btn") + page.click("#bump-btn") + page.wait_for_timeout(500) + expect(page.locator("#counter")).to_have_text("0") + assert len(reports) == 1 diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 7b5c000295e..17c86420829 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -17,6 +17,7 @@ from unittest.mock import AsyncMock, Mock import pytest +import reflex_base from pytest_mock import MockerFixture from reflex_base.components.component import Component from reflex_base.constants.state import FIELD_MARKER @@ -42,7 +43,7 @@ import reflex as rx from reflex import AdminDash, constants from reflex._upload import upload -from reflex.app import App, ComponentCallable, default_overlay_component +from reflex.app import App, ComponentCallable, EventNamespace, default_overlay_component from reflex.compiler.compiler import ( _compile_app, _memoize_stateful_app_wraps, @@ -3910,3 +3911,256 @@ def register_route(self, *, add_page, **_): assert "from-plugin" in app._unevaluated_pages assert "from-plugin" in app._pages assert app._plugin_routes_registered + + +@pytest.fixture +def event_namespace() -> EventNamespace: + """An EventNamespace with a mock app and one linked client session. + + Returns: + The event namespace. + """ + namespace = EventNamespace(namespace="/_event", app=Mock()) + namespace.sid_to_token["known_sid"] = "some_token" + return namespace + + +@pytest.fixture +def frontend_errors(event_namespace: EventNamespace) -> list[str]: + """Capture exceptions routed to the app's frontend exception handler. + + Args: + event_namespace: The event namespace. + + Returns: + The captured exception messages. + """ + errors: list[str] = [] + event_namespace.app.frontend_exception_handler = lambda exc: errors.append(str(exc)) + return errors + + +@pytest.fixture +def client_error_console(monkeypatch: pytest.MonkeyPatch) -> dict[str, list[str]]: + """Capture messages logged through the console helpers. + + Args: + monkeypatch: The pytest monkeypatch fixture. + + Returns: + Captured messages keyed by log level. + """ + captured: dict[str, list[str]] = {"error": [], "warn": [], "debug": []} + for level in captured: + monkeypatch.setattr( + console, + level, + lambda msg, _level=level, **kwargs: captured[_level].append(msg), + ) + return captured + + +@pytest.mark.asyncio +async def test_client_error_dispatch_missing_reports_actionable_error( + event_namespace: EventNamespace, frontend_errors: list[str] +): + """A dispatch_function_missing error reports the substate and remediation steps. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + """ + await event_namespace.on_client_error( + "known_sid", + { + "error_type": constants.ClientErrorType.DISPATCH_MISSING, + "message": "Cannot process state update", + "substate": "reflex___state____state.my___state____my_state", + }, + ) + assert len(frontend_errors) == 1 + message = frontend_errors[0] + assert "reflex___state____state.my___state____my_state" in message + assert "rebuild" in message.lower() + + +@pytest.mark.asyncio +async def test_client_error_generic_reports_type_and_message( + event_namespace: EventNamespace, frontend_errors: list[str] +): + """A generic client error reports the error type and message. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + """ + await event_namespace.on_client_error( + "known_sid", + { + "error_type": constants.ClientErrorType.STATE_UPDATE, + "message": "boom", + }, + ) + assert len(frontend_errors) == 1 + message = frontend_errors[0] + assert constants.ClientErrorType.STATE_UPDATE in message + assert "boom" in message + + +@pytest.mark.asyncio +@pytest.mark.parametrize("payload", ["not a dict", None, ["list"], 42]) +async def test_client_error_malformed_payload_is_ignored( + event_namespace: EventNamespace, + frontend_errors: list[str], + payload: Any, +): + """Non-dict payloads are dropped without raising or reporting errors. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + payload: The malformed payload to send. + """ + await event_namespace.on_client_error("known_sid", payload) + assert not frontend_errors + + +@pytest.mark.asyncio +async def test_client_error_unknown_sid_does_not_report_error( + event_namespace: EventNamespace, + frontend_errors: list[str], + client_error_console: dict[str, list[str]], +): + """Errors from sockets without a linked token are not reported. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + client_error_console: Captured console messages. + """ + await event_namespace.on_client_error( + "unknown_sid", + { + "error_type": constants.ClientErrorType.STATE_UPDATE, + "message": "spam from unauthenticated socket", + }, + ) + assert not frontend_errors + assert not client_error_console["error"] + + +@pytest.mark.asyncio +async def test_client_error_values_are_sanitized_and_truncated( + event_namespace: EventNamespace, frontend_errors: list[str] +): + """Control characters are stripped and long messages truncated before reporting. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + """ + evil = "\x1b[31mINJECT\x1b[0m\nFAKE LOG LINE\t" + "A" * 5000 + await event_namespace.on_client_error( + "known_sid", + {"error_type": "custom_type", "message": evil}, + ) + assert len(frontend_errors) == 1 + message = frontend_errors[0] + assert "\x1b" not in message + assert "\n" not in message + assert "\t" not in message + assert len(message) < 700 + + +@pytest.mark.asyncio +async def test_client_error_reporting_is_rate_limited_per_sid( + event_namespace: EventNamespace, frontend_errors: list[str] +): + """A single session cannot flood the backend logs with error reports. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + """ + for _ in range(20): + await event_namespace.on_client_error( + "known_sid", + {"error_type": "custom_type", "message": "spam"}, + ) + assert len(frontend_errors) == EventNamespace._MAX_CLIENT_ERRORS_PER_SID + # Disconnecting removes the counter so the mapping cannot grow unboundedly. + task = event_namespace.on_disconnect("known_sid") + if task is not None: + await task + assert "known_sid" not in event_namespace._client_error_counts + + +@pytest.mark.asyncio +async def test_client_error_reporting_bounded_across_reconnects( + event_namespace: EventNamespace, + frontend_errors: list[str], + client_error_console: dict[str, list[str]], +): + """Reconnecting with fresh SIDs does not grant an unlimited report budget. + + Args: + event_namespace: The event namespace. + frontend_errors: Captured frontend exception handler messages. + client_error_console: Captured console messages. + """ + for reconnect in range(50): + sid = f"sid_{reconnect}" + event_namespace.sid_to_token[sid] = f"token_{reconnect}" + for _ in range(5): + await event_namespace.on_client_error( + sid, {"error_type": "custom_type", "message": "spam"} + ) + assert len(frontend_errors) == EventNamespace._MAX_CLIENT_ERRORS_PER_WINDOW + # Suppression is not silent: one warning is logged when the cap trips, so + # a flooding client cannot invisibly starve reports from other sessions. + assert ( + len([ + msg for msg in client_error_console["warn"] if "suppressing" in msg.lower() + ]) + == 1 + ) + # Once the window elapses, errors are reported again (not silenced forever). + event_namespace._client_error_window_start -= ( + EventNamespace._CLIENT_ERROR_WINDOW_SECONDS + 1 + ) + event_namespace.sid_to_token["sid_fresh"] = "token_fresh" + await event_namespace.on_client_error( + "sid_fresh", {"error_type": "custom_type", "message": "after window"} + ) + assert len(frontend_errors) == EventNamespace._MAX_CLIENT_ERRORS_PER_WINDOW + 1 + + +def test_client_error_event_name_matches_handler(): + """python-socketio dispatches events to on_ methods by naming + convention; this pins the handler to SocketEvent.CLIENT_ERROR. + """ + assert ( + f"on_{constants.SocketEvent.CLIENT_ERROR}" + == EventNamespace.on_client_error.__name__ + ) + + +def test_client_error_constants_match_frontend(): + """The socket event and error types are duplicated as literals in state.js. + + Nothing at runtime keeps the two definitions in sync, so pin them here. + """ + state_js = ( + Path(reflex_base.__file__).parent / ".templates/web/utils/state.js" + ).read_text() + assert ( + f'const CLIENT_ERROR_EVENT = "{constants.SocketEvent.CLIENT_ERROR}"' in state_js + ) + assert ( + f'const ERROR_TYPE_DISPATCH_MISSING = "{constants.ClientErrorType.DISPATCH_MISSING}"' + in state_js + ) + assert ( + f'const ERROR_TYPE_STATE_UPDATE = "{constants.ClientErrorType.STATE_UPDATE}"' + in state_js + ) diff --git a/tests/units/utils/test_format.py b/tests/units/utils/test_format.py index c7ae6397020..0e6515a6f49 100644 --- a/tests/units/utils/test_format.py +++ b/tests/units/utils/test_format.py @@ -832,3 +832,55 @@ def test_format_library_name(input: str, output: str): ) def test_json_dumps(input, output): assert format.json_dumps(input) == output + + +def test_sanitize_client_log_value_respects_max_length(): + """The sanitized value never exceeds max_length, even when truncated.""" + out = format.sanitize_client_log_value("A" * 5000, max_length=500) + assert len(out) <= 500 + assert out.endswith("... (truncated)") + + +def test_sanitize_client_log_value_strips_control_characters(): + """Control characters cannot be used to forge extra backend log lines.""" + out = format.sanitize_client_log_value("\x1b[31mred\x1b[0m\nFAKE LOG LINE\tx") + assert "\x1b" not in out + assert "\n" not in out + assert "\t" not in out + + +@pytest.mark.parametrize( + "payload", + [ + "x[/bold]y", + "x[/]y", + "[blink bold red]FAKE", + "[link=https://evil.example]z[/link]", + ], +) +def test_sanitize_client_log_value_escapes_markup(payload: str): + """Client-supplied rich markup is escaped so printing it cannot raise. + + Args: + payload: The markup payload a client could send. + """ + from reflex_base.utils import console + + # Must not raise MarkupError. + console.error(f"[Frontend Error] {format.sanitize_client_log_value(payload)}") + + +def test_sanitize_client_log_value_bounds_work_before_scanning(): + """Only max_length characters are scanned, however long the input is.""" + scanned = 0 + + class CountingStr(str): + def __getitem__(self, item: Any): + nonlocal scanned + result = super().__getitem__(item) + if isinstance(item, slice): + scanned = len(result) + return result + + format.sanitize_client_log_value(CountingStr("A" * 100_000), max_length=500) + assert scanned == 500