Repository navigation
fix: rehydrate state trees that expire during a background task - #7585
FarhanAliRaza wants to merge 4 commits into
Conversation
A background task releases the state lock while it runs. If the session expires in that time, `async with self` gets a recreated tree, and the delta carries only the handler's changes. The browser keeps the values of the expired session. A recreated `rx.State` root has empty `router_data`. A foreground event already uses this signal to rehydrate. `StateProxy.__aenter__` now uses the same signal: it restores the router from the event context and runs `BaseStateEventProcessor._rehydrate`. The hydrate snapshot carries `is_hydrated=false`, so the browser keeps its client storage. Closes reflex-dev#7478
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
[Medium impact] Fixes state rehydration when background tasks resume after session expiry. The PR still needs the old-route recovery issue fixed before merging. Findings
SummaryBackground tasks now rehydrate loaded states when they re-enter an expired session.
Reviews (3) · Last reviewed commit: "test: disable oplock when simulating Red..." · Reviewed by Greptile |
| if ctx.router_data: | ||
| mutable_state.router_data = mutable_state._update_router_vars( | ||
| ctx.router_data, {} | ||
| ) | ||
| await BaseStateEventProcessor._rehydrate(mutable_state) |
There was a problem hiding this comment.
ctx.router_data belongs to the event that started the background task, not necessarily the page the user is viewing now. If the task starts on /a, the user visits /b, and the session then expires before the task resumes, this code restores /a and sends its router values to the browser. _rehydrate also selects /a’s load handlers.
Recover the current browser route before rehydrating instead of treating the task’s old route as current.
Mark the proxy mutable before the rehydrate, so a failure or a cancellation goes through __aexit__ and closes the state manager context.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0593158a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Merging this PR will not alter performance
|
Oplock keeps the tree cached in the process, so deleting the Redis keys does not expire it.
Summary
A background task releases the state lock while it runs. If the session expires in that time,
async with selfgets a recreated tree. The delta then carries only the handler's changes, and the browser keeps the values of the expired session (#7478).A foreground event already recovers from this. When the
rx.Stateroot has emptyrouter_data,_execute_eventruns_rehydrate. This PR makesStateProxy.__aenter__use the same signal:BaseStateEventProcessor._rehydrate, now astaticmethodbecause it did not useself.The hydrate snapshot carries
is_hydrated=false, so the frontend does not write client storage. The router, the dynamic route args andrx.LocalStorage/rx.Cookievalues stay correct. The result is the same as a foreground event onmaintoday.Supersedes #7543. That PR tracks recreated states in every manager and adds a Redis manifest key. This PR needs no new tracking state, and it also covers a Redis tree that expired completely.
Closes #7478
Test plan
test_background_task_rehydrates_state_tree_expired_while_unlocked, for the memory and Redis managers. It fails onmainand passes with this change.tests/units/istate,tests/units/reflex_base,test_state.py,test_event.pyandtest_app.pypass. The istate and event processor tests also pass against a live Redis.ComponentState, anrx.LocalStoragevar and a dynamic route. The background task sleeps longer than the token TTL.pid, the router,is_hydratedand the browser's localStorage are kept.pidand localStorage are kept.Known limit, the same as the foreground path on
main: on Redis,hydratesends only the states loaded in the tree, so a sibling state that the task does not load keeps its old value.