Skip to content

Commit 52a351c

Browse files
authored
Gracefully reset the StateProxy internals on error (#6169)
* Gracefully reset the StateProxy internals on error If we catch an error during `StateProxy.__aenter__`, we have to manually unwind the internal state changes to allow the proxy to be used again. * go back to try/finally in StateProxy.__aexit__
1 parent 1b39a5a commit 52a351c

File tree

3 files changed

+66
-15
lines changed

3 files changed

+66
-15
lines changed

reflex/istate/proxy.py

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import functools
99
import inspect
1010
import json
11+
import sys
1112
from collections.abc import Callable, Sequence
1213
from importlib.util import find_spec
1314
from types import MethodType
@@ -132,15 +133,20 @@ async def __aenter__(self) -> Self:
132133
raise ImmutableStateError(msg)
133134

134135
await self._self_actx_lock.acquire()
135-
self._self_actx_lock_holder = current_task
136-
self._self_actx = self._self_app.modify_state(
137-
token=self._self_substate_token, background=True
138-
)
139-
mutable_state = await self._self_actx.__aenter__()
140-
super().__setattr__(
141-
"__wrapped__", mutable_state.get_substate(self._self_substate_path)
142-
)
143-
self._self_mutable = True
136+
try:
137+
self._self_actx_lock_holder = current_task
138+
self._self_actx = self._self_app.modify_state(
139+
token=self._self_substate_token, background=True
140+
)
141+
mutable_state = await self._self_actx.__aenter__()
142+
self._self_mutable = True
143+
super().__setattr__(
144+
"__wrapped__", mutable_state.get_substate(self._self_substate_path)
145+
)
146+
except (Exception, asyncio.CancelledError):
147+
# Restore the proxy to a consistent state since __aexit__ will not be called when __aenter__ raises.
148+
await self.__aexit__(*sys.exc_info())
149+
raise
144150
return self
145151

146152
async def __aexit__(self, *exc_info: Any) -> None:
@@ -154,15 +160,14 @@ async def __aexit__(self, *exc_info: Any) -> None:
154160
if self._self_parent_state_proxy is not None:
155161
await self._self_parent_state_proxy.__aexit__(*exc_info)
156162
return
157-
if self._self_actx is None:
158-
return
159-
self._self_mutable = False
160163
try:
161-
await self._self_actx.__aexit__(*exc_info)
164+
if self._self_mutable and self._self_actx is not None:
165+
await self._self_actx.__aexit__(*exc_info)
162166
finally:
167+
self._self_actx = None
168+
self._self_mutable = False
163169
self._self_actx_lock_holder = None
164170
self._self_actx_lock.release()
165-
self._self_actx = None
166171

167172
def __enter__(self):
168173
"""Enter the regular context manager protocol.

tests/units/conftest.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ def app_module_mock(monkeypatch) -> mock.Mock:
4646
return app_module_mock
4747

4848

49+
@pytest.fixture
50+
def mock_app(app_module_mock: mock.Mock, app: App) -> App:
51+
"""A mocked dummy app per test.
52+
53+
Args:
54+
app_module_mock: The mock for the main app module.
55+
app: A default App instance.
56+
57+
Returns:
58+
The mock app instance.
59+
"""
60+
app_module_mock.app = app
61+
return app
62+
63+
4964
@pytest.fixture(scope="session")
5065
def windows_platform() -> bool:
5166
"""Check if system is windows.

tests/units/istate/test_proxy.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,14 @@
22

33
import dataclasses
44
import pickle
5+
from asyncio import CancelledError
6+
from contextlib import asynccontextmanager
7+
from unittest.mock import patch
8+
9+
import pytest
510

611
import reflex as rx
7-
from reflex.istate.proxy import MutableProxy
12+
from reflex.istate.proxy import MutableProxy, StateProxy
813

914

1015
@dataclasses.dataclass
@@ -35,3 +40,29 @@ def test_mutable_proxy_pickle_preserves_object_identity():
3540
assert unpickled["direct"][0].id == 1
3641
assert unpickled["proxied"][0].id == 1
3742
assert unpickled["direct"][0] is unpickled["proxied"][0]
43+
44+
45+
@pytest.mark.usefixtures("mock_app")
46+
@pytest.mark.asyncio
47+
async def test_state_proxy_recovery():
48+
"""Ensure that `async with self` can be re-entered after a lock issue."""
49+
state = ProxyTestState()
50+
state_proxy = StateProxy(state)
51+
52+
with patch("reflex.app.App.modify_state") as mock_modify_state:
53+
54+
@asynccontextmanager
55+
async def mock_modify_state_context(*args, **kwargs): # noqa: RUF029
56+
msg = "Simulated lock issue"
57+
raise CancelledError(msg)
58+
yield
59+
60+
mock_modify_state.side_effect = mock_modify_state_context
61+
62+
with pytest.raises(CancelledError, match="Simulated lock issue"):
63+
async with state_proxy:
64+
pass
65+
66+
# After the exception, we should be able to enter the context again without issues
67+
async with state_proxy:
68+
pass

0 commit comments

Comments
 (0)