Skip to content

fix(state): persist cancelled redis handler updates - #7412

Merged
FarhanAliRaza merged 5 commits into
reflex-dev:mainfrom
kr1shna-exe:fix/7248-persist-cancelled-state
Oct 30, 2026
Merged

FarhanAliRaza merged 5 commits into
reflex-dev:mainfrom
kr1shna-exe:fix/7248-persist-cancelled-state

Conversation

@kr1shna-exe

@kr1shna-exe kr1shna-exe commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #7248.

When a foreground handler is cancelled by a superseding event, Redis can discard every state change it made before cancellation, including updates already yielded to the client. The replacement handler then reads the old state. Preserve those changes before releasing the Redis lock so the replacement sees the same history as it does with in-memory state.

What changed

  • Share cancellation-aware persistence across the three non-cached Redis locking paths. Save under the existing lock fence and re-raise cancellation afterward.
  • Cover root and substate writes, handler cleanup, cached leases, expired locks and non-cancellation exceptions.
  • Exercise superseding handlers through the event processor and check both stored state and emitted deltas. Document the behavior and add a news fragment.

Verification

  • Reproduced three failing manager cases and the failing Redis event-processor case before the fix using an isolated Redis server.
  • uv run pytest tests/units --cov --no-cov-on-fail --cov-report=: 11,193 passed, 155 skipped; 78.68% coverage.
  • Redis manager tests against real Redis: 27 passed. Event-processor regression with real Redis and the in-memory control: 2 passed.
  • uv run ruff check ., uv run ruff format --check . and uv run pyright reflex tests passed.
  • All pre-commit hooks applicable to the five changed files passed.
  • All-files pre-commit passed formatting, lint, spelling, stub generation and generated-source checks. Its Pyright hook found six existing macOS CPU-affinity errors in unchanged benchmark files; those files pass with --pythonplatform Linux, matching CI. The remaining ty and Biome hooks passed separately.

Review in cubic

@kr1shna-exe
kr1shna-exe requested review from a team and Alek99 as code owners October 4, 2026 16:46

@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 5 files

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issue was found.

Summary

Preserves Redis state changes when foreground handlers fail or are cancelled.

  • Redis keeps state changes when a handler is cancelled or raises.

The earlier repeated-cancellation finding is addressed: another cancellation no longer interrupts the separate save task.

Reviews (5) · Last reviewed commit: "fix(state): keep redis writes of handler..." · Reviewed by Greptile

Comment thread reflex/istate/manager/redis.py Resolved
@masenf masenf added bug Something isn't working documentation Improvements or additions to documentation labels Oct 6, 2026
@FarhanAliRaza FarhanAliRaza added the on deck PRs lined up to review / merge next label Oct 22, 2026
@codspeed

codspeed Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 4.49%

1 warning

⚠️ 18 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚡ 1 improved benchmark
✅ 147 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_route_argument_extraction 119.6 µs 114.5 µs +4.49%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing kr1shna-exe:fix/7248-persist-cancelled-state (6df0011) with main (96dca47)

Open in CodSpeed

Comment thread docs/events/yield_events.md Resolved
A handler exception dropped its writes on the non-cached Redis paths,
while memory, disk and the Redis cached-lease path keep them. Save on
any handler exception, not only on cancellation, so every path matches.

Document that a write after the last yield reaches the frontend only
with a later update on Redis.
@FarhanAliRaza

FarhanAliRaza commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

I pushed two commits to this branch.

  1. f032c88a4 merges main. fix(state-manager): stop redis lock handoffs from stalling events until the lock expires #7534 merged the contended path and the lease-ended path into one single_update block. _modify_state_with_lock now wraps the two non-cached paths: oplock disabled and single_update.
  2. 6df001171 keeps the writes of a handler that raises, not only of a cancelled handler.

Why the second change: on Redis, a handler exception dropped its writes on the non-cached paths. Memory, disk and the Redis cached-lease path keep them. So the result depended on whether a lease was cached. _modify_state_with_lock now saves on (Exception, asyncio.CancelledError). test_modify_exception_does_not_persist_state is now test_modify_exception_persists_state.

The docs now also say that on Redis, a write made after the last yield is stored, but the frontend receives it only with a later update to the same var, or after a page reload.

If the lock expired, the fenced save writes nothing and raises LockExpiredError. For a cancelled handler, the result is now an error toast in place of a quiet cancel. This happens only when the handler held the lock longer than lock_expiration, and a handler that completes already reports the same error in that case.

@FarhanAliRaza
FarhanAliRaza merged commit aeea15a into reflex-dev:main Oct 30, 2026
169 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation on deck PRs lined up to review / merge next

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A cancelled foreground supersedes=True handler loses all of its state writes under StateManagerRedis but keeps them under the in-memory manager

3 participants