From 4500595ebbd1ee10e607a706dd1dc524e1fbabb4 Mon Sep 17 00:00:00 2001 From: Gaurav Dubey Date: Thu, 22 Oct 2026 12:20:28 +0000 Subject: [PATCH 1/2] fix(state): log disk state files that fail to load instead of ignoring them StateManagerDisk.load_state swallowed every exception, so an existing state file that could not be read looked exactly like a missing one: get_state started from a fresh state and the next write replaced the file, with nothing in the logs. A damaged or unreadable file is now logged as a warning with the traceback. A state class that changed since the file was written is still an expected reset (as in the redis manager) and is logged at debug level. BaseState reports both as StateSchemaMismatchError, so the unpickling cause tells them apart. Closes #7167 --- news/+disk-state-load-errors.bugfix.md | 1 + reflex/istate/manager/disk.py | 30 ++++++- tests/units/istate/manager/test_disk.py | 101 ++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 news/+disk-state-load-errors.bugfix.md diff --git a/news/+disk-state-load-errors.bugfix.md b/news/+disk-state-load-errors.bugfix.md new file mode 100644 index 00000000000..a7abdf21421 --- /dev/null +++ b/news/+disk-state-load-errors.bugfix.md @@ -0,0 +1 @@ +The disk state manager now logs a warning when a persisted state file exists but cannot be loaded, instead of silently starting from a fresh state. diff --git a/reflex/istate/manager/disk.py b/reflex/istate/manager/disk.py index 920e19867fb..f06539e3ce6 100644 --- a/reflex/istate/manager/disk.py +++ b/reflex/istate/manager/disk.py @@ -5,6 +5,7 @@ import dataclasses import functools import logging +import pickle import time from collections.abc import AsyncIterator from hashlib import md5 @@ -12,6 +13,7 @@ from typing import Any, Generic, cast from reflex_base.environment import state_manager_disk_debounce +from reflex_base.utils.exceptions import StateSchemaMismatchError from typing_extensions import Unpack, override from reflex.istate.manager import ( @@ -146,6 +148,11 @@ def token_path(self, token: StateToken) -> Path: async def load_state(self, token: StateToken[TOKEN_TYPE]) -> TOKEN_TYPE | None: """Load a state object based on the provided token. + A missing state file returns None without logging. A state file that + cannot be loaded also returns None: a state class that changed since the + file was written is logged at debug level, a damaged or unreadable file + as a warning. + Args: token: The token used to identify the state object. @@ -158,8 +165,27 @@ async def load_state(self, token: StateToken[TOKEN_TYPE]) -> TOKEN_TYPE | None: try: with token_path.open(mode="rb") as file: return token.deserialize(fp=file) - except Exception: - pass + except Exception as err: + # BaseState reports a damaged file as a schema mismatch too, so + # tell it apart from a state class that changed by its cause. + if isinstance(err, StateSchemaMismatchError) and not isinstance( + err.__cause__, (pickle.UnpicklingError, EOFError) + ): + logger.debug( + "Discarding persisted state for %s from %s: it no longer " + "matches the state class.", + token, + token_path, + exc_info=True, + ) + return None + logger.warning( + "Failed to load persisted state for %s from %s, starting " + "from a fresh state.", + token, + token_path, + exc_info=True, + ) return None async def populate_substates( diff --git a/tests/units/istate/manager/test_disk.py b/tests/units/istate/manager/test_disk.py index 402a14cb31e..5b0021afe1f 100644 --- a/tests/units/istate/manager/test_disk.py +++ b/tests/units/istate/manager/test_disk.py @@ -1,11 +1,14 @@ """Tests for the disk state manager.""" +import logging import math import os +import pickle from pathlib import Path import pytest +from reflex.istate.manager import disk from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.token import BaseStateToken, StateToken from reflex.state import BaseState @@ -167,3 +170,101 @@ def recording_with_cls(self, cls): assert written == [DiskTreeFirst] await state_manager.close() + + +@pytest.mark.asyncio +async def test_load_state_missing_file_is_not_logged(tmp_path, monkeypatch, caplog): + """A state that was never persisted loads as None without logging. + + Args: + tmp_path: A temporary directory. + monkeypatch: The pytest monkeypatch fixture. + caplog: The pytest log capture fixture. + """ + monkeypatch.setattr(prerequisites, "get_states_dir", lambda: tmp_path) + state_manager = StateManagerDisk(_write_debounce_seconds=0) + token = StateToken(ident="client", cls=int) + + with caplog.at_level(logging.DEBUG, logger=disk.__name__): + assert await state_manager.load_state(token) is None + + assert not caplog.records + await state_manager.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("token", "contents", "fresh_state"), + [ + (StateToken(ident="client", cls=int), b"not-a-pickle", 0), + (StateToken(ident="client", cls=int), b"", 0), + (BaseStateToken(ident="client", cls=DiskPersistState), b"not-a-pickle", None), + (BaseStateToken(ident="client", cls=DiskPersistState), b"", None), + ], +) +async def test_load_state_logs_unreadable_file( + tmp_path, monkeypatch, caplog, token, contents, fresh_state +): + """A persisted state file that is damaged is logged as a warning. + + Args: + tmp_path: A temporary directory. + monkeypatch: The pytest monkeypatch fixture. + caplog: The pytest log capture fixture. + token: The token whose state file is damaged. + contents: The damaged file contents. + fresh_state: The expected fresh state, or None for a DiskPersistState. + """ + monkeypatch.setattr(prerequisites, "get_states_dir", lambda: tmp_path) + state_manager = StateManagerDisk(_write_debounce_seconds=0) + token_path = state_manager.token_path(token) + token_path.write_bytes(contents) + + with caplog.at_level(logging.DEBUG, logger=disk.__name__): + assert await state_manager.load_state(token) is None + + [record] = caplog.records + assert record.levelno == logging.WARNING + assert str(token_path) in record.getMessage() + assert record.exc_info is not None + + caplog.clear() + with caplog.at_level(logging.WARNING, logger=disk.__name__): + state = await state_manager.get_state(token) + assert [r.levelno for r in caplog.records] == [logging.WARNING] + if fresh_state is None: + assert isinstance(state, DiskPersistState) + assert math.isclose(state.num, 3.15) + else: + assert state == fresh_state + await state_manager.close() + + +@pytest.mark.asyncio +async def test_load_state_logs_schema_mismatch_at_debug(tmp_path, monkeypatch, caplog): + """A persisted BaseState whose class changed since it was written is logged at debug. + + Args: + tmp_path: A temporary directory. + monkeypatch: The pytest monkeypatch fixture. + caplog: The pytest log capture fixture. + """ + monkeypatch.setattr(prerequisites, "get_states_dir", lambda: tmp_path) + state_manager = StateManagerDisk(_write_debounce_seconds=0) + token = BaseStateToken(ident="client", cls=DiskPersistState) + token_path = state_manager.token_path(token) + stale_state = DiskPersistState(_reflex_internal_init=True) # pyright: ignore [reportCallIssue] + stale_state.num = 9.5 + token_path.write_bytes(pickle.dumps(("stale-schema", stale_state))) + + with caplog.at_level(logging.DEBUG, logger=disk.__name__): + assert await state_manager.load_state(token) is None + + [record] = caplog.records + assert record.levelno == logging.DEBUG + assert str(token_path) in record.getMessage() + + state = await state_manager.get_state(token) + assert isinstance(state, DiskPersistState) + assert math.isclose(state.num, 3.15) + await state_manager.close() From bf6cee21436330b0433f3a49cef2a9ed2650b976 Mon Sep 17 00:00:00 2001 From: Gaurav Dubey Date: Thu, 22 Oct 2026 12:47:21 +0000 Subject: [PATCH 2/2] fix(state): warn on any disk state file that fails to unpickle Treat a StateSchemaMismatchError as an expected reset only when it has no chained cause. BaseState._deserialize chains every unpickling failure, so a pickle referencing a missing module is now a warning too, not only a truncated or garbled file. Also assert the token in the warning and tighten the changelog wording. --- news/+disk-state-load-errors.bugfix.md | 2 +- reflex/istate/manager/disk.py | 16 ++++++---------- tests/units/istate/manager/test_disk.py | 11 ++++++++++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/news/+disk-state-load-errors.bugfix.md b/news/+disk-state-load-errors.bugfix.md index a7abdf21421..50c6d4c1886 100644 --- a/news/+disk-state-load-errors.bugfix.md +++ b/news/+disk-state-load-errors.bugfix.md @@ -1 +1 @@ -The disk state manager now logs a warning when a persisted state file exists but cannot be loaded, instead of silently starting from a fresh state. +The disk state manager now logs a warning when an existing persisted state file cannot be read or unpickled, instead of silently starting from a fresh state. A file whose stored schema no longer matches the state class is still reset, and logged at debug level. diff --git a/reflex/istate/manager/disk.py b/reflex/istate/manager/disk.py index f06539e3ce6..defa1b3bde6 100644 --- a/reflex/istate/manager/disk.py +++ b/reflex/istate/manager/disk.py @@ -5,7 +5,6 @@ import dataclasses import functools import logging -import pickle import time from collections.abc import AsyncIterator from hashlib import md5 @@ -149,9 +148,9 @@ async def load_state(self, token: StateToken[TOKEN_TYPE]) -> TOKEN_TYPE | None: """Load a state object based on the provided token. A missing state file returns None without logging. A state file that - cannot be loaded also returns None: a state class that changed since the - file was written is logged at debug level, a damaged or unreadable file - as a warning. + cannot be loaded also returns None: one whose stored schema no longer + matches the state class is logged at debug level, one that cannot be + read or unpickled as a warning. Args: token: The token used to identify the state object. @@ -166,17 +165,14 @@ async def load_state(self, token: StateToken[TOKEN_TYPE]) -> TOKEN_TYPE | None: with token_path.open(mode="rb") as file: return token.deserialize(fp=file) except Exception as err: - # BaseState reports a damaged file as a schema mismatch too, so - # tell it apart from a state class that changed by its cause. - if isinstance(err, StateSchemaMismatchError) and not isinstance( - err.__cause__, (pickle.UnpicklingError, EOFError) - ): + # BaseState also reports a file it cannot unpickle as a schema + # mismatch, chained to the unpickling error. + if isinstance(err, StateSchemaMismatchError) and err.__cause__ is None: logger.debug( "Discarding persisted state for %s from %s: it no longer " "matches the state class.", token, token_path, - exc_info=True, ) return None logger.warning( diff --git a/tests/units/istate/manager/test_disk.py b/tests/units/istate/manager/test_disk.py index 5b0021afe1f..1f1ff564de4 100644 --- a/tests/units/istate/manager/test_disk.py +++ b/tests/units/istate/manager/test_disk.py @@ -14,6 +14,9 @@ from reflex.state import BaseState from reflex.utils import prerequisites +# A pickle of a global from a module that does not exist. +MISSING_MODULE_PICKLE = b"cmissing_module_for_disk_test\nThing\n." + class DiskPersistState(BaseState): """A state for testing disk persistence.""" @@ -200,12 +203,17 @@ async def test_load_state_missing_file_is_not_logged(tmp_path, monkeypatch, capl (StateToken(ident="client", cls=int), b"", 0), (BaseStateToken(ident="client", cls=DiskPersistState), b"not-a-pickle", None), (BaseStateToken(ident="client", cls=DiskPersistState), b"", None), + ( + BaseStateToken(ident="client", cls=DiskPersistState), + MISSING_MODULE_PICKLE, + None, + ), ], ) async def test_load_state_logs_unreadable_file( tmp_path, monkeypatch, caplog, token, contents, fresh_state ): - """A persisted state file that is damaged is logged as a warning. + """A persisted state file that cannot be read or unpickled is logged as a warning. Args: tmp_path: A temporary directory. @@ -225,6 +233,7 @@ async def test_load_state_logs_unreadable_file( [record] = caplog.records assert record.levelno == logging.WARNING + assert str(token) in record.getMessage() assert str(token_path) in record.getMessage() assert record.exc_info is not None