Skip to content

fix(sqlite): report lock contention as retryable - #550

Open
chuiwenwei wants to merge 1 commit into
EverMind-AI:mainfrom
chuiwenwei:fix/sqlite-busy-1.6
Open

chuiwenwei wants to merge 1 commit into
EverMind-AI:mainfrom
chuiwenwei:fix/sqlite-busy-1.6

Conversation

@chuiwenwei

Copy link
Copy Markdown

Summary

A SQLite write that waits out busy_timeout raises OperationalError and
used to reach the catch-all handler as a 500. Classify only lock/busy
errors as DatabaseBusyError and map them to 503 with Retry-After: 1;
every other operational error still propagates unchanged. The session
scope rolls the transaction back first, so nothing is committed.

Closes #498.

Area

  • Architecture method
  • Benchmark
  • Use case
  • Documentation
  • Developer experience
  • CI, build, or release

Verification

make lint        passed (14 contracts kept)
make test        3094 passed, 4 skipped
make integration 201 passed, 9 skipped, 7 deselected

Additional tests:

  • test_session_scope_maps_lock_contention_to_database_busy holds a real
    BEGIN IMMEDIATE lock and asserts DatabaseBusyError.
  • test_session_scope_leaves_other_operational_errors_unchanged proves a
    missing-table error is not misclassified.
  • test_locked_sqlite_returns_503_then_succeeds drives the production
    handler: 503 DATABASE_BUSY + Retry-After, then 200 after release.

Checklist

  • I kept the change scoped to the relevant area.
  • I am opening this from a separate branch, not pushing directly to main.
  • I updated docs, examples, or setup notes when behavior changed.
  • I added or updated tests when the change affects behavior.
  • I did not commit secrets, .env files, dependency folders, or generated output.
  • Active relative links in Markdown files resolve.

Notes for Reviewers

  • I kept a dedicated DATABASE_BUSY code because the response has a
    concrete retry hint; if you prefer reusing EXTERNAL_SERVICE_UNAVAILABLE,
    it is a one-line change plus test updates.
  • DatabaseBusyError stays under StorageError; cascade retry
    classification is out of scope for this PR.
  • The lock check prefers sqlite_errorcode & 0xFF and falls back to the
    database is locked message for drivers that do not expose the code.

By submitting this pull request, I agree that my contribution is licensed under
the Apache License 2.0.

A SQLite write that waits out busy_timeout raises OperationalError and
used to reach the catch-all handler as a 500. Classify only lock/busy
errors as DatabaseBusyError and map it to 503 with Retry-After: 1; every
other operational error still propagates unchanged.

The session scope rolls the transaction back before raising, so nothing
is committed. Add a real BEGIN IMMEDIATE lock test, an unrelated
OperationalError test, and an HTTP integration test that proves 503 then
200 after the lock is released.

Refs EverMind-AI#498.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flush returns 500 when SQLite write hits the busy timeout

1 participant