-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
Describe the bug
If an exception is raised inside async with self block, changes prior to the exception are not persisted, but the backend still sends a delta with the changes. Subsequent events or async with self blocks do NOT see the changes from the block that raised.
To Reproduce
import reflex as rx
class State(rx.State):
count: int = 0
@rx.event
def increment(self):
self.count += 1
@rx.event
def increment_with_error(self):
self.count += 1
raise ValueError("This is an error!")
@rx.event(background=True)
async def increment_with_error_bg(self):
async with self:
self.count += 1
raise ValueError("This is an error in the background task!")
@rx.event(background=True)
async def increment_with_error_in_context_bg(self):
async with self:
self.count += 1
raise ValueError("This is an error in the background task async with self ctx!")
def index() -> rx.Component:
return rx.container(
rx.color_mode.button(position="top-right"),
rx.vstack(
rx.button(
f"Increment ({State.count})",
on_click=State.increment,
),
rx.button(
"Increment with error",
on_click=State.increment_with_error,
),
rx.button(
"Increment with error in background task",
on_click=State.increment_with_error_bg,
),
rx.button(
"Increment with error in background task async with self ctx",
on_click=State.increment_with_error_in_context_bg,
),
),
)
app = rx.App()
app.add_page(index)Expected behavior
When any of the four buttons are clicked, the counter should be incremented. When refreshing the page, the count should remain the same.
What actually happens when clicking the fourth button "Increment with error in background task async with self ctx" is the count temporarily increases, the default error toast is shown, then refreshing the page causes the count to revert back to the prior value.
Specifics (please complete the following information):
- Python Version: 3.14
- Reflex Version: 0.8.26
Additional Context
This bug is kind of a weird edge case.
The bug behavior described here makes sense semantically if we consider async with self to be a "transaction" in that we take the lock, yield control to the block, then persist the changes, send updates, and release the lock. If you understand that, then it makes sense why raising the exception inside the context block could end up not persisting the changes (although it's also a bit weird that we do get the update 🤔).
But from a uniformity perspective, i think it's important to not have a behavior difference between memory/disk and redis state managers.
So either redis papers over this difference and persists regardless of an exception in the block, or we do something with memory/disk manager such that we always give a copy of the state to the event and require set_state to be called for those implementations. Ultimately it might be better for the memory/disk managers to work more like the redis manager to help find bugs like this in local dev mode, but that would probably be a breaking 0.9 change.