Skip to content

fix: never exit the process on an unhandled database rejection - #516

Merged
pdp2121 merged 3 commits into
mainfrom
fix-connections-crashloop
Oct 2, 2026
Merged

pdp2121 merged 3 commits into
mainfrom
fix-connections-crashloop

Conversation

@pdp2121

@pdp2121 pdp2121 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Stops vhs-connections crash-looping when the Knex pool saturates.

  • Install an unhandledRejection handler in all three services (connection-manager, crawler, api) so a failed database call is logged instead of exiting the process
  • saveValidator: catch the second statement, which had no error handler
  • Catch the five fire-and-forget call sites in wsHandling.ts and agreement.ts
  • Bound the Knex pool explicitly (min: 2, max: 20) — the knex default of 10 was the ceiling
  • Drop acquireConnectionTimeout from 120s to 30s so a backlog surfaces instead of queueing

Context of Change

  • vhs-connections was restarting 90–144 times per 24h across dev, staging and prod, with reason=Error (not OOMKilled)
  • saveValidator's second statement had no .catch():
    await query('validators')
      .where({ signing_key: validator.signing_key, revoked: null })
      .update({ revoked: false })      // no handler
  • Its caller is fire-and-forget — void agreement.handleValidation(...) — so a KnexTimeoutError propagated out as an unhandled rejection. Since Node 15 that throws, which exits the process
  • A scan found the same shape at 7 more void sites and 6 unguarded timer callbacks, all calling functions with no internal try/catch. Patching each individually would leave the next unfound path to crash the same way, so the global handler is the actual fix and the targeted .catch() calls just keep errors attributable in logs
  • The exhausted pool is Knex's client-side pool, not Postgres. RDS was at 23 of 838 connections, so raising max uses headroom that already exists
  • uncaughtException is deliberately not trapped — a synchronous throw can leave the process in an indeterminate state where continuing is worse than restarting

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Before / After

  • Before: one transient KnexTimeoutError → unhandled rejection → process exit → k8s restart → repeat
  • After: the error is logged, the service stays up, and the next validation or job run retries
  • Verified directly: an unhandled rejection exits with code 1 without the guard; with it, the rejection is logged and the process continues and exits 0
  • Pool ceiling raised from 10 to 20; a saturated pool now fails in 30s rather than queueing for 2 minutes

@pdp2121 pdp2121 changed the title Fix connections crashloop fix: never exit the process on an unhandled database rejection Oct 2, 2026
@pdp2121
pdp2121 merged commit 7c51c5c into main Oct 2, 2026
3 checks passed
@pdp2121
pdp2121 deleted the fix-connections-crashloop branch October 2, 2026 16:13
cybele-ripple added a commit that referenced this pull request Oct 6, 2026
#519)

## High Level Overview of Change
`calculateAgreement()` runs one promise per validator inside a single
`Promise.all`. If any single validator's promise rejected, the whole
`Promise.all` rejected immediately, which skipped the post-batch cleanup
(`purgeHourlyAgreementScores()`, `chains.purgeChains()`, and the
`reported_at` advance) for every validator that hour, not just the one
that failed.

Catch each validator's promise individually inside
`calculateAgreement()`'s loop, so the batch-level purge/reset always
runs regardless of a single validator's failure
Add a `.catch` to `getHourlyAgreementScores` (the one DB read in
`shared/database/agreement.ts` with no error handling, unlike every
write in that file)

### Context of Change
validator-history-service tracks per-validator agreement scores on an
hourly cycle. After scoring all validators for the hour, it's supposed
to clear its in-memory ledger-chain tracking so the next hour starts
clean.


[#516](#516)
(merged 2026-10-02) added an `unhandledRejection` handler so a Knex pool
timeout no longer crashes the process. That fix surfaced this issue:
`getHourlyAgreementScores` can still throw on a transient DB error, and
since it was awaited inside one of the per-validator promises in
`calculateAgreement()`'s Promise.all, a single rejection aborted the
whole batch's cleanup step — not just that one validator's score.

With the cleanup skipped, each chain's accumulated ledger set never gets
cleared and keeps growing hour over hour, while the validations window
(`validationsByPublicKey`) stays bounded to 2 hours. That mismatch is
what produces the observed symptom in prod: agreement scores that look
stuck or wrong, with missed growing relative to a roughly flat validated
count.

### Type of Change

<!--
Please check relevant options, delete irrelevant ones.
-->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactor (non-breaking change that only restructures code)
- [x] Tests (You added tests for code that already exists, or your new
feature included in this PR)
- [ ] Documentation Updates
- [ ] Release

## Before / After
- **Before**: one validator's agreement calculation failing (e.g. a
transient DB read error) silently skipped `purgeHourlyAgreementScores()`
/ `chains.purgeChains()` / the `reported_at` reset for the entire batch
that hour, letting chain state accumulate unbounded over time.
- **After**: a single validator's failure is logged and contained to
that validator; the batch-level purge and `reported_at` advance run
every hour regardless.

## Test Plan
Current tests pass and added
`test/database/agreement-score-errors.test.ts`,
`test/chains/agreement-batch-isolation.test.ts`
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.

2 participants