Skip to content

fix(state): retry 'no more rows available' across all sqlite3.Error classes - #74934

Closed
Dannou wants to merge 1 commit into
NousResearch:mainfrom
Dannou:fix/session-persistence-retry-catchall
Closed

fix(state): retry 'no more rows available' across all sqlite3.Error classes#74934
Dannou wants to merge 1 commit into
NousResearch:mainfrom
Dannou:fix/session-persistence-retry-catchall

Conversation

@Dannou

@Dannou Dannou commented Jul 30, 2026

Copy link
Copy Markdown

PR Draft — fix(state): retry "no more rows available" across all sqlite3.Error classes

Repo upstream: https://github.com/NousResearch/hermes-agent
Branche locale: fix/session-persistence-retry-catchall (commit fd8a9612c)
Diff: hermes_state.py +48/−4


Symptom

On Windows dual-writer setups (a standalone messaging gateway process + a desktop-spawned serve process sharing the same state.db in WAL mode), conversation turns sporadically die with:

Session DB append_message failed: no more rows available
Turn ended: reason=session_persistence_failed

and the user-facing "session storage could not be written" stop message. Always on large appends (17–26 KB tool results, which trigger heavy FTS5 trigram sync work inside the messages INSERT triggers), always under concurrent write load.

Root cause

_execute_write's retry net handles the transient SQLite engine error "no more rows available" (documented in the existing comment as dual gateway/agent WAL contention) only when it surfaces as sqlite3.OperationalError or sqlite3.DatabaseError.

On this SQLite build (uv CPython 3.11, bundled sqlite3.dll), the error surfaces as a class that lives outside DatabaseError (most likely InterfaceError), so it propagates on attempt 0 without any retry.

Timing proof from production logs: the append failed 1.16 s after the last tool result, while 60 retries at the minimum 20 ms jitter take ≥ 1.18 s — the retry loop was provably never entered.

The existing code comment already acknowledges "exact subclass varies with the SQLite build" — the net was just one branch short.

Fix

Add a catch-all except sqlite3.Error branch (after the OperationalError and DatabaseError branches) that applies the same jittered retry when the message contains "no more rows", and re-raises anything else unchanged.

Also includes the local dual-writer hardening the retry constants needed:

  • _WRITE_MAX_RETRIES: 15 → 60 (1.3 s tolerance lost races against dual-writer FTS sync on large appends)
  • _WRITE_RETRY_MAX_S: 150 ms → 300 ms
  • engine timeout: 1.0 s → 2.0 s

Verification

  • 3/3 targeted unit tests on _execute_write:
    • InterfaceError("no more rows available") on first attempt → retried, succeeds on attempt 2
    • InterfaceError with a different message → propagates immediately, 1 call only (no behavior change)
    • OperationalError("database is locked") → retried as before (no regression)
  • python -m py_compile hermes_state.py clean
  • Production soak: zero session_persistence_failed since the patched backend started (previously 3 occurrences in 24 h)

Reproduction notes

  • Standalone single-writer: identical SQL always succeeds (verified against a production-copy 2.5 GB state.db, 32 KB payloads, triggers firing).
  • Dual-writer stress (2 writers × 26 KB trigram-heavy payloads + a concurrent checkpoint/FTS-optimize loop): massive database is locked contention (all correctly OperationalError); the no more rows race is rarer and build-dependent — hence the catch-all rather than a class-specific branch.

Out of scope

  • The dual-gateway topology itself (documented as supported); this PR only makes the write path resilient to it.
  • FTS shadow-table corruption salvage (_try_runtime_fts_rebuild) — untouched.

Checklist avant publication (reste à faire avec validation utilisateur)

  • gh auth status — compte GitHub actif (publication publique sous l'identité de l'utilisateur)
  • Fork NousResearch/hermes-agentDannou/hermes-agent
  • Push branche fix/session-persistence-retry-catchall sur le fork
  • gh pr create avec ce corps de description
  • Vérifier que la CI upstream passe

…lasses

Dual gateway/agent WAL writers contend on large appends (FTS5 trigram sync
holds the write lock). Under contention the SQLite engine raises the
transient 'no more rows available' error; on some Windows builds it
surfaces as InterfaceError, which lives OUTSIDE DatabaseError and escaped
both existing retry branches (observed 2026-07-30: turn killed 1.16s into
the append < 1.18s minimum duration for 60 retries => retry loop bypassed).

- add catch-all 'except sqlite3.Error' retry branch for 'no more rows'
- keep prior local hardening: 60 write retries (was 15), 20-300ms jitter,
  2.0s engine timeout (was 1.0s)

Verified: 3/3 unit tests (InterfaceError retried, other InterfaceError
still propagates immediately, OperationalError locked unchanged).

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the exception hierarchy. Current main still has the relevant gap: _execute_write retries only OperationalError messages containing locked/busy (hermes_state.py:2369-2398), while its following DatabaseError branch is for FTS-corruption recovery (hermes_state.py:2399-2411). An InterfaceError with the reported message would escape both paths.

Problems

  • The branch conflicts with current main because commit 8da8a7887d06373e169af6e431ec52ebb439ec7a replaced _WRITE_MAX_RETRIES with deadline-based write patience (hermes_state.py:1757-1763, 2346-2387). The proposed fixed-attempt loops need to be ported to that current mechanism.
  • The PR changes only hermes_state.py; no regression test covers the claimed InterfaceError case.

Suggested changes

  • Add the narrow retry classification while preserving current patience_s deadlines and fast/slow jitter.
  • Add tests for retrying InterfaceError("no more rows available") and immediately propagating unrelated InterfaceError messages.

Automated hermes-sweeper review.

Comment thread hermes_state.py
@@ -2640,6 +2667,23 @@ def _execute_write(
if not self._try_runtime_fts_rebuild(exc):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please port this catch-all to current main's deadline-based retry policy. Commit 8da8a78 replaced _WRITE_MAX_RETRIES with patience_s and fast/slow jitter, so this fixed-attempt branch cannot be salvaged verbatim without discarding the newer transcript-specific wait budget.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history platform/windows Native Windows-specific behavior or breakage P1 High — major feature broken, no workaround sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 30, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
teknium1 pushed a commit that referenced this pull request Aug 1, 2026
…e3.Error classes

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 (8da8a78): 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.
teknium1 pushed a commit that referenced this pull request Aug 1, 2026
…e3.Error classes

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 (8da8a78): 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.
teknium1 pushed a commit that referenced this pull request Aug 1, 2026
…e3.Error classes

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 (8da8a78): 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.
@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merged via salvage PR #75883 (#75883). Your diff predated the deadline-patience rewrite (8da8a78), so we ported the substantive piece — a message-scoped 'no more rows available' retry that catches ALL sqlite3.Error classes including InterfaceError — onto the current patience loop, authored under your name. The constants changes were superseded by the rewrite. Fixes the gap you identified. Thanks for the Windows dual-writer analysis!

randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…e3.Error classes

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 NousResearch#74934 onto the deadline-patience rewrite (12b687a): 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants