Skip to content

Fix reflex run hanging after fatal error in the frontend thread - #6990

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

Fix reflex run hanging after fatal error in the frontend thread#6990
masenf merged 1 commit into
mainfrom
claude/rel-fix-node-error-hang

Conversation

@masenf

@masenf masenf commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

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-011 (MEDIUM) from the 0.9.9a1 pre-release testing round: reflex run hangs indefinitely after a fatal node-version error on the npm path. With an outdated node first on PATH and REFLEX_USE_NPM=1, js_runtimes.validate_frontend_dependencies correctly prints "Reflex requires node version 22.22.0 or higher..." and raises SystemExit(1) — but that happens inside the run_frontend task submitted to a ThreadPoolExecutor by processes.run_concurrently_context. The SystemExit only kills the worker thread: the futures are checked after the with-body, and in dev mode the main thread blocks forever serving the backend (granian serve()), so the error is swallowed and the process never exits (it kept the backend serving with no frontend until killed by timeout). The same swallowing applies to any fatal error raised by an executor task consumed through this helper.

Fix

run_concurrently_context now surfaces task failures to the main thread:

  • A done-callback on each future interrupts the main thread when a task fails while the with-body is executing. On POSIX this must be a real signal (signal.pthread_kill(main_thread, SIGINT)) so a blocking C call (lock wait, server loop) returns with EINTR and runs the SIGINT handler — _thread.interrupt_main() only sets a pending flag there and never wakes a blocked main thread. On Windows interrupt_main() is the right mechanism (it sets the SIGINT event that blocking waits monitor).
  • The delivered SIGINT either triggers the backend server's own graceful-shutdown handler (granian/uvicorn), letting the existing post-body as_completed check re-raise the task's exception, or raises KeyboardInterrupt in the body, which the context manager now catches and converts back into the failed task's own exception. A genuine Ctrl+C with no failed task propagates unchanged.
  • A task that already failed before the body starts is re-raised up front so the main thread never enters (and blocks in) the body; only one interrupt is ever sent per context, and no interrupt is sent once the body has finished, so run_concurrently (empty body) keeps its existing error semantics.

Test plan

  • New regression test tests/units/utils/test_processes.py::test_run_concurrently_context_unblocks_main_thread_on_task_failure: a task raising SystemExit while the with-body blocks on an Event.wait(timeout=10) must propagate promptly. Against unfixed main it fails (the body blocks the full 10s before the error surfaces — the unit-scale version of the infinite hang); with the fix it passes in milliseconds. Two companion tests cover genuine KeyboardInterrupt passthrough and run_concurrently exception propagation.
  • 300-iteration in-process stress of the failure path (instant and delayed task failure, blocked body, and the empty-body run_concurrently path): no hang, no escaped KeyboardInterrupt.
  • End-to-end repro from the finding: fresh blank app, fake node shim printing v22.12.0, REFLEX_USE_NPM=1 reflex run — unfixed main runs until killed (timeout exit 124 after 90s); with the fix the process prints the node version error, "Reflex app stopped.", and exits code 1 in ~5s.
  • uv run ruff check . and uv run ruff format . clean; uv run pyright reflex tests 0 errors; uv run pytest tests/units/utils passes (629 passed; the only 2 failures are pre-existing on clean main in this sandbox, which lacks IPv6).

🤖 Generated with Claude Code

https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x


Generated by Claude Code

Review in cubic

A SystemExit (e.g. the node minimum-version check failing on the npm
path) raised inside the run_frontend task only killed its worker thread:
run_concurrently_context checked its futures after the with-body, but in
dev mode the main thread blocks indefinitely serving the backend, so the
error was swallowed and reflex run never exited.

run_concurrently_context now interrupts the main thread when a task
fails while the with-body is executing - a real SIGINT via pthread_kill
on POSIX so blocking C calls return with EINTR, interrupt_main on
Windows - and re-raises the task's own exception so the CLI exits
promptly with the original error. A task that fails before the body
starts is surfaced without entering the body, and a genuine Ctrl+C with
no failed task still propagates unchanged.

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 18:31
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates concurrent development-server startup so a fatal frontend worker failure interrupts the blocking main thread and is re-raised promptly.

  • Adds platform-specific main-thread interruption and failed-future propagation.
  • Adds regression coverage for blocked bodies, genuine keyboard interrupts, and ordinary concurrent exceptions.
  • Adds a bug-fix news entry.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking clarity issue in the new timing assertion.

The changed concurrency path has focused coverage for worker failures and genuine interrupts; the only accepted concern is that the regression test leaves its five-second threshold unexplained.

Files Needing Attention: tests/units/utils/test_processes.py

Important Files Changed

Filename Overview
reflex/utils/processes.py Adds synchronized failed-task detection and main-thread interruption while preserving genuine keyboard interrupts.
tests/units/utils/test_processes.py Adds focused concurrency regression tests; the elapsed-time assertion should explain its five-second threshold.
news/+node-error-hang.bugfix.md Documents the corrected CLI behavior when a fatal frontend worker error occurs.

Reviews (1): Last reviewed commit: "Fix reflex run hanging after fatal error..." | Re-trigger Greptile

Comment thread tests/units/utils/test_processes.py
@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-node-error-hang (e6ab4ee) with main (fba9cc5)

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.

@masenf masenf added this to the v0.9.9 milestone Aug 28, 2026

@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.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread reflex/utils/processes.py
Comment thread reflex/utils/processes.py
@masenf
masenf merged commit 8afe8ce into main Aug 28, 2026
111 checks passed
@masenf
masenf deleted the claude/rel-fix-node-error-hang branch August 28, 2026 19:43
masenf pushed a commit that referenced this pull request Aug 28, 2026
…-context-refactor-jv3pig

Picks up #6987, #6990, #6989, #6992 and #6993. Clean auto-merge; main's
``templates.py`` edit is again in the Vite config template, away from
``render_iterable_tag``.
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.

3 participants