Skip to content

Handle payload-less client_error emits without raising TypeError - #6984

Merged
masenf merged 1 commit into
mainfrom
claude/rel-fix-client-error-noarg
Aug 28, 2026
Merged

Handle payload-less client_error emits without raising TypeError#6984
masenf merged 1 commit into
mainfrom
claude/rel-fix-client-error-noarg

Conversation

@masenf

@masenf masenf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Defect

FINDING-004 (security-adjacent, medium) from the 0.9.9a1 pre-release testing batch: EventNamespace.on_client_error(self, sid, data) in reflex/app.py gave data no default. python-socketio dispatches events as handler(sid, *data[1:]), so an emit("client_error") with no payload calls on_client_error(sid) and raises TypeError: missing 1 required positional argument: 'data' during parameter binding — before the malformed-payload check, the unknown-SID gate, and the per-SID/per-window rate limiters introduced by #6827 ever run. The exception surfaces via asyncio's "Task exception was never retrieved" handler, so any connected socket — including one that never linked a valid token — could flood the backend logs with tracebacks at default loglevel, unsuppressable via --loglevel, defeating the anti-abuse hardening the handler carries.

Fix

Default data to None. The existing isinstance(data, dict) guard then drops a payload-less emit at debug level exactly like any other malformed payload, so every anti-abuse guard runs first. Docstring updated to explain why the default exists; news fragment news/+client-error-noarg.bugfix.md added.

Test plan

  • New regression test tests/units/test_app.py::test_client_error_no_argument_emit_is_ignored (parametrized over known/unknown SID) simulates socketio's no-argument dispatch by calling on_client_error(sid) with no data, asserting no exception, no report through frontend_exception_handler, and no error/warning log records. It fails on unfixed main with the exact TypeError from the finding and passes with the fix.
  • All 14 client_error tests and the full tests/units/test_app.py (145 tests) pass.
  • uv run ruff check ., uv run ruff format ., and uv run pyright reflex tests (0 errors) are clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x


Generated by Claude Code

Review in cubic

python-socketio dispatches events as handler(sid, *data[1:]), so an
emit("client_error") with no payload calls on_client_error(sid) and
raised TypeError during parameter binding -- before the malformed-payload,
unknown-SID, and rate-limit guards ever ran. The exception surfaced as an
asyncio "Task exception was never retrieved" traceback, letting any
connected socket (no valid token needed) flood backend logs at default
loglevel, defeating the anti-abuse hardening the handler carries.

Give data a default of None so the existing isinstance(data, dict) guard
drops the payload-less emit at debug level like any other malformed
payload. Found as FINDING-004 in 0.9.9a1 pre-release testing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
@masenf
masenf requested a review from a team as a code owner August 28, 2026 17:54
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes EventNamespace.on_client_error accept payload-less Socket.IO events and routes the missing payload through the existing malformed-payload guard.

  • Defaults the optional client-error payload to None.
  • Documents why payload-less dispatch requires the default.
  • Adds regression coverage for known and unknown session IDs.
  • Adds a bug-fix news fragment.

Confidence Score: 5/5

The PR appears safe to merge because payload-less client-error events now reach the existing malformed-input guard without changing valid-payload handling.

The signature-only behavioral change converts an omitted argument into None, and the existing first guard returns immediately for that value; focused regression tests cover both known and unknown session IDs.

Important Files Changed

Filename Overview
reflex/app.py Safely defaults the client-error payload to None, which the existing dictionary guard immediately rejects without triggering reporting.
tests/units/test_app.py Adds focused regression coverage confirming payload-less calls neither raise nor produce frontend reports or warning/error console output.
news/+client-error-noarg.bugfix.md Accurately documents the payload-less dispatch failure and the resulting behavior.

Reviews (1): Last reviewed commit: "Default client_error payload to None so ..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Aug 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/rel-fix-client-error-noarg (5cc3029) with main (f7c848f)2

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (45b8ed5) during the generation of this report, so f7c848f was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

No issues found across 3 files

Re-trigger cubic

@masenf
masenf merged commit fba9cc5 into main Aug 28, 2026
111 checks passed
@masenf
masenf deleted the claude/rel-fix-client-error-noarg branch August 28, 2026 18:18
masenf pushed a commit that referenced this pull request Aug 28, 2026
…-context-refactor-jv3pig

Picks up #6984, #6967 and #6985 — a client_error payload default and two
deprecation-shim restorations. Clean auto-merge; nothing touches client state,
memoization, or foreach.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants