From 14eca89779e5d005b4f38a1dbbbeca2c044e7f51 Mon Sep 17 00:00:00 2001 From: Dannoob Date: Fri, 31 Jul 2026 21:56:30 -0700 Subject: [PATCH] fix(state): retry transient 'no more rows available' across all sqlite3.Error classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under dual gateway/agent WAL contention (FTS5 trigram sync holding the write lock on large appends) the SQLite engine can raise a transient 'no more rows available' error. The exception CLASS varies with the build — some surface it as InterfaceError, a SIBLING of DatabaseError — so it escaped both existing retry branches in _execute_write on attempt 0 and killed the turn as session_persistence_failed even though the identical write succeeds standalone. Port of #74934 onto the deadline-patience rewrite (8da8a7887d): the PR's attempt-counted constants (60 retries / 300ms jitter / 2.0s engine timeout) predate that rewrite and are superseded by the patience budget, so they are intentionally NOT carried over. Instead the check is message-scoped and rides the existing deadline/patience loop: - extract the jittered-sleep-within-deadline logic into a shared _sleep_before_retry helper (behavior-preserving for locked/busy) - retry 'no more rows available' from OperationalError, DatabaseError (checked BEFORE the FTS-corruption rebuild path so it is not misrouted), and a message-scoped sqlite3.Error catch-all - any other error in any class propagates untouched on attempt 0 Tests: transient InterfaceError retried to success; unrelated InterfaceError propagates immediately; DatabaseError variant retried; exhausted patience surfaces the original error. --- hermes_state.py | 26 +++++++- tests/state/test_no_more_rows_retry.py | 92 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 tests/state/test_no_more_rows_retry.py diff --git a/hermes_state.py b/hermes_state.py index 02b9cf0558efc..22e2d052676d6 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -2405,6 +2405,17 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) # Set on the first compression-busy collision so the short wait is # measured from then, not from the start of the write. compression_deadline: Optional[float] = None + + # Transient engine-level error observed on contended WAL appends + # (dual gateway/agent writers; FTS5 trigram sync holds the write + # lock). The identical write succeeds standalone, so it is + # retryable like locked/busy. The exception CLASS varies with the + # SQLite build — some surface it as InterfaceError, which lives + # OUTSIDE DatabaseError and escaped the retry net entirely on + # attempt 0 — so the check is message-scoped, not class-scoped. + def _is_no_more_rows(exc: sqlite3.Error) -> bool: + return "no more rows available" in str(exc).lower() + while True: try: with self._lock: @@ -2460,9 +2471,13 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) "a large WAL checkpoint, or an older pre-update " "process; the database itself is healthy)" ) from exc - # Non-lock error — propagate. + if _is_no_more_rows(exc) and _sleep_before_retry(): + continue + # Non-lock error or patience exhausted — propagate. raise except sqlite3.DatabaseError as exc: + if _is_no_more_rows(exc) and _sleep_before_retry(): + continue # Corrupt FTS shadow tables make every write raise the # malformed/corrupt error class through the FTS sync triggers # while the canonical messages table is intact. The gateway @@ -2475,6 +2490,15 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) if not self._try_runtime_fts_rebuild(exc): raise continue + except sqlite3.Error as exc: + # Catch-all for builds that surface 'no more rows available' + # as InterfaceError (a sibling of DatabaseError, not a + # subclass) or another sqlite3.Error class outside the two + # handlers above. Message-scoped: anything else propagates + # untouched. + if _is_no_more_rows(exc) and _sleep_before_retry(): + continue + raise def _sleep_before_write_retry( self, deadline: float, patience_s: float diff --git a/tests/state/test_no_more_rows_retry.py b/tests/state/test_no_more_rows_retry.py new file mode 100644 index 0000000000000..753293d66f807 --- /dev/null +++ b/tests/state/test_no_more_rows_retry.py @@ -0,0 +1,92 @@ +"""Retry of transient 'no more rows available' engine errors (#74934 port). + +Under dual gateway/agent WAL contention (FTS5 trigram sync holding the +write lock on large appends), the SQLite engine can raise a transient +'no more rows available' error. The exception CLASS varies with the +SQLite build — some surface it as ``sqlite3.InterfaceError``, which is a +sibling of ``DatabaseError`` (not a subclass) and therefore escaped both +existing retry branches in ``_execute_write`` on attempt 0, killing the +turn as ``session_persistence_failed`` while the identical write would +have succeeded milliseconds later. + +The fix is message-scoped, not class-scoped: any ``sqlite3.Error`` whose +text contains 'no more rows available' retries within the existing +deadline/patience loop; every other error propagates untouched. +""" + +import sqlite3 + +import pytest + +from hermes_state import SessionDB + + +@pytest.fixture +def db(tmp_path, monkeypatch): + # Keep retries fast: tiny jitter, short-but-sufficient patience. + monkeypatch.setattr(SessionDB, "_WRITE_PATIENCE_S", 2.0) + monkeypatch.setattr(SessionDB, "_WRITE_RETRY_MIN_S", 0.001) + monkeypatch.setattr(SessionDB, "_WRITE_RETRY_MAX_S", 0.005) + d = SessionDB(db_path=tmp_path / "state.db") + yield d + d.close() + + +class TestNoMoreRowsRetry: + def test_transient_interface_error_is_retried_to_success(self, db): + """InterfaceError('no more rows available') must be retried inside + the deadline/patience loop and succeed once the contention clears.""" + calls = {"n": 0} + + def flaky(conn): + calls["n"] += 1 + if calls["n"] <= 3: + raise sqlite3.InterfaceError("no more rows available") + conn.execute( + "INSERT INTO state_meta (key, value) VALUES ('nmr', 'ok') " + "ON CONFLICT(key) DO UPDATE SET value=excluded.value" + ) + return "done" + + assert db._execute_write(flaky) == "done" + assert calls["n"] == 4 + assert db.get_meta("nmr") == "ok" + + def test_unrelated_interface_error_propagates_immediately(self, db): + """The catch-all is message-scoped: an InterfaceError with any other + text must escape on the first attempt, not be swallowed/retried.""" + calls = {"n": 0} + + def broken(conn): + calls["n"] += 1 + raise sqlite3.InterfaceError("bad parameter or other API misuse") + + with pytest.raises(sqlite3.InterfaceError, match="bad parameter"): + db._execute_write(broken) + assert calls["n"] == 1 + + def test_no_more_rows_via_database_error_is_retried(self, db): + """Some builds raise the same transient message through the generic + DatabaseError class — it must ride the same retry loop instead of + being misrouted into the FTS-corruption rebuild path.""" + calls = {"n": 0} + + def flaky(conn): + calls["n"] += 1 + if calls["n"] <= 2: + raise sqlite3.DatabaseError("no more rows available") + return "ok" + + assert db._execute_write(flaky) == "ok" + assert calls["n"] == 3 + + def test_exhausted_patience_propagates_the_transient_error(self, db, monkeypatch): + """If contention never clears within the patience budget, the + original error must surface rather than looping forever.""" + monkeypatch.setattr(SessionDB, "_WRITE_PATIENCE_S", 0.05) + + def always(conn): + raise sqlite3.InterfaceError("no more rows available") + + with pytest.raises(sqlite3.InterfaceError, match="no more rows"): + db._execute_write(always)