Repository navigation
feat: detect event loop blocking in development - #7672
harsh21234i wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
[Medium impact] The warnings affect event processing in development apps. The code appears safe to merge once the new test helper meets the repository’s docstring requirement. Findings
SummaryAdds development-time warnings for event handlers that block the asyncio loop.
Reviews (11) · Last reviewed commit: "feat: disable blocking detection with 0 ..." · Reviewed by Greptile |
Merging this PR will degrade performance by 4.2%1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Name the event task running on the blocked loop instead of tracking handler frames per event, so blocking in computed vars after the handler is reported. Log after the loop responds with the real duration. Remove the per-event hooks and the duplicated otel branch, and simplify the threshold validation.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Capture the loop thread's frames from the event task's entry to the blocking line when the threshold is crossed, so the warning points at the code that blocked, including computed vars and C-extension calls.
Log the handler and its stack as soon as the threshold is reached, so a handler that never returns is reported, then log the total duration once the loop responds. Report nothing when the frame snapshot no longer contains the event task's entry frame.
Use 0 instead of None to turn detection off, so REFLEX_EVENT_LOOP_BLOCKING_THRESHOLD=0 works: the env parser cannot produce None for optional fields. Name the innermost frame of app code (under the app root, outside the stdlib and installed packages) in the first warning line, so library frames do not hide the line to fix.
| async def _execute_event(self: EventProcessor, **kwargs): | ||
| await execute_event(self, **kwargs) | ||
| time.sleep(0.12) # noqa: ASYNC251 |
There was a problem hiding this comment.
The new _execute_event helper has no docstring. The repository guide requires Google-style docstrings on all functions, including nested test helpers. Add a short summary explaining that this helper simulates blocking after the handler returns. This requirement must be satisfied before merging.
| async def _execute_event(self: EventProcessor, **kwargs): | |
| await execute_event(self, **kwargs) | |
| time.sleep(0.12) # noqa: ASYNC251 | |
| async def _execute_event(self: EventProcessor, **kwargs): | |
| """Simulate blocking after the event handler returns.""" | |
| await execute_event(self, **kwargs) | |
| time.sleep(0.12) # noqa: ASYNC251 |
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
It is test code, but CLAUDE.md requires Google-style docstrings on all functions and doesn’t exempt test helpers. Since this nested function simulates blocking after the handler returns, the one-line docstring is still appropriate.
| if task is None or not (name := task.get_name()).startswith(EVENT_TASK_PREFIX): | ||
| return None |
There was a problem hiding this comment.
we probably should still report blocking for any task, not just reflex_event| tasks. that way if there is an error with the lock waiter or the lost and found subscriber, those can still be surfaced here instead of detecting the blocking, but not reporting anything. The message can be slightly different when we detect blocking in an arbitrary task.
| assert ( | ||
| "is blocking the event loop at " | ||
| "tests/units/reflex_base/event/processor/test_event_processor.py:" | ||
| ) in caplog.text |
There was a problem hiding this comment.
use platform-aware path separators.
| assert ( | |
| "is blocking the event loop at " | |
| "tests/units/reflex_base/event/processor/test_event_processor.py:" | |
| ) in caplog.text | |
| exp_path = Path("tests", "units", "reflex_base", "event", "processor", "test_event_processor.py") | |
| assert ( | |
| f"is blocking the event loop at {path}:" | |
| ) in caplog.text |
Summary
Warn in development when an event blocks the asyncio event loop.
event_loop_blocking_thresholdseconds (default0.2), the loop is blocked.asyncio.current_task(loop)and gets the handler from the event task's name (reflex_event|<event>|…). This covers the whole event, including computed vars recalculated after the handler.event_loop_blocking_threshold=0, orREFLEX_EVENT_LOOP_BLOCKING_THRESHOLD=0, to turn it off.Example:
Closes #7619
Testing
0, and the env var.asynchandler, a background event, a helper function and a slow computed var. All 11 are detected with the correct handler and app line, on Python 3.11 and 3.14, with asyncio, uvloop and rloop. There were no warnings for awaiting handlers or blocks shorter than the threshold.