Skip to content

fix: contain background runner store failures - #12

Open
charan-rathore wants to merge 1 commit into
CopilotKit:mainfrom
charan-rathore:fix-runner-tick-store-errors
Open

charan-rathore wants to merge 1 commit into
CopilotKit:mainfrom
charan-rathore:fix-runner-tick-store-errors

Conversation

@charan-rathore

Copy link
Copy Markdown

What this fixes

The runner's scheduler tick called store.claim() outside the try block. If the store threw (for example a transient SQLite lock or error), tick() rejected. Because it runs from an immediate/interval callback and the server has no unhandledRejection handler, that rejection could take down the always-on server.

Changes

  • Catch store errors at the tick level so later ticks keep running and can pick up other work.
  • If the ownership timer hits a DB error while research is running, abort that research cleanly.
  • The existing finally still clears the active slot and timers, even when persisting a failure also throws.
  • No new retry behavior: a job whose failure could not be saved keeps the existing lease-recovery policy.

Tests

Two regression tests for claim() and fail() errors, plus an ownership timer test. They fail on the base commit and pass with the change. They use the real SQLite store and inject the exceptions, so they cover the failure paths deterministically. I did not test real lock contention.

Results below are from the person who prepared the change; I did not rerun them. Full suite: 34 files, 159 tests pass. Lint, typecheck, format check and production build pass (the build prints a chunk size warning that was already there). No dependency changes.

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.

1 participant