fix(backend): add graceful degradation for RPC outages (#1162) - #1208
Open
Awosdot wants to merge 18 commits into
Open
fix(backend): add graceful degradation for RPC outages (#1162)#1208Awosdot wants to merge 18 commits into
Awosdot wants to merge 18 commits into
Conversation
…tartup EventPollingService.start() let a failed startup event replay abort before the continuous polling loop was armed, so a transient Soroban RPC outage during boot would permanently disable event polling until the process was restarted. Catch the replay failure, log a clear warning, and continue starting the poll loop — the next successful poll re-derives the same missed range from the persisted cursor, so nothing is lost.
|
@Awosdot Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
added 17 commits
August 24, 2026 21:14
npm ci failed on main with "Missing: @yieldvault/api-schemas@1.0.0 from lock file" because the lockfile's link-package entry for the local ../packages/api-schemas file dependency was missing the lockfileVersion 3 "link" metadata npm ci requires. Regenerated via `npm install --package-lock-only`; no dependency versions changed.
- eventOutbox.test.ts / sloMetrics.test.ts: use const for never-reassigned bindings (prefer-const) - optimisticConcurrency.ts / walletAliasService.ts: rewrite while(true) retry loops as for(;;), which is not flagged by no-constant-condition
…erride migration schema.prisma had three models (EventOutbox, AdminConfigChange, FeatureFlagOverride) and a `version` column on BulkExportJob/VaultState with no corresponding migration, so `prisma migrate deploy` against a fresh database left those tables missing. This is what made most of the backend test suite fail with "table does not exist" errors. Generated the reconciling migration via `prisma migrate dev --create-only` and dropped its one cosmetic index-rename step (dropping an index that backs a UNIQUE constraint isn't supported directly on SQLite; the existing autoindex already enforces the same constraint). Regenerated dev.db from the full, now-complete migration history.
…L bug ScopedAdminTokenStore is fully async/Prisma-backed, but the Issue Junirezz#723 tests in issues705-707-719-723.test.ts called create()/revoke()/rotate()/ list()/authenticate()/clear() without awaiting them and asserted on the unresolved promises. Two of those tests wrapped the async create() call in a synchronous expect(() => ...).toThrow(), which turned validation errors into unhandled promise rejections that crashed the whole Jest worker process. Made the describe block properly async/await throughout and switched the two throw assertions to expect(...).rejects.toThrow(). Separately, getStalePendingTtlMs() in writeAheadAuditLog.ts treated an explicit WAL_STALE_PENDING_TTL_MS=0 (used by a test to mean "everything is immediately stale") as unset and fell back to the 15-minute default, because the guard was `parsed > 0` instead of `parsed >= 0`.
Bumped axios, body-parser, brace-expansion, fast-uri, js-yaml, and protobufjs to their patched versions within existing semver ranges (package.json unchanged). Reduces npm audit findings from 31 (6 high) to 25 (2 high) with no code changes required. The remaining 2 high-severity findings are all in the @opentelemetry/* dependency chain, which has no non-breaking fix available — resolving those needs a major-version upgrade of the OpenTelemetry SDK and is left out of scope here since it would require re-validating the tracing instrumentation in tracing.ts / index.ts.
… findings Forced the last 2 high-severity npm audit findings (both rooted in @opentelemetry/core <2.8.0) by bumping @opentelemetry/sdk-node, exporter-trace-otlp-http, and instrumentation-http from ^0.218.0 to ^0.221.0, which pulls in a patched @opentelemetry/core transitively. `npm audit` now reports 0 vulnerabilities. Verified: `npm run build` and `npm run lint` are clean, all previously affected test suites still pass, and a runtime smoke test of initTracing()/withSpan()/shutdownTracing() from tracing.ts behaves identically to before the bump (the only error observed is an expected ECONNREFUSED from the exporter trying to flush spans to a real OTLP collector, which isn't running in this environment and is unrelated to the version change).
…ion string
schema.prisma's datasource provider is "sqlite", so its generated client
can only accept file: URLs no matter what override is passed in. The
backend-governance CI job sets DATABASE_URL to a Postgres connection
string for the separate raw `pg` pool in database.ts (which has its own
migrations under scripts/postgres-migrations.js), but prisma.ts was also
reading DATABASE_URL and passing it straight through to PrismaClient's
datasource override, which Prisma rejects outright ("the URL must start
with the protocol file:") — crashing every Prisma-backed query in that
job before a single test could run.
buildDatasourceUrl() now falls back to the schema-declared sqlite file
whenever DATABASE_URL isn't itself a file: URL, instead of forwarding an
override the generated client can never accept.
fix(ci): pin cargo-audit to a version compatible with the pinned Rust toolchain
rust-toolchain.toml pins the workspace to rustc 1.85.0 (for reproducible
Soroban/WASM contract builds), but `cargo install cargo-audit` was
installing the latest release (0.22.2), which requires rustc 1.88+.
Pinned to cargo-audit 0.22.1, which the error message itself confirms
supports 1.85, instead of bumping the toolchain pin and risking a change
in contract build output.
…safety false positive The EventOutbox migration's auto-generated SQLite table rebuild (drop + recreate) for adding a version column to BulkExportJob/VaultState tripped the repo's migration-safety check, which hard-bans any DROP TABLE/COLUMN/ INDEX pattern with no annotation opt-out. Rewrote it as plain ALTER TABLE ADD COLUMN statements (fully supported by SQLite for a column with a constant DEFAULT), which achieves the same schema without ever dropping the table or its existing indexes. Also fixed a real false positive in the migration-safety script itself: its ADD COLUMN NOT NULL / no DEFAULT check anchored the regex match at "NOT NULL", so a trailing "DEFAULT x" (Prisma's own convention — "TYPE NOT NULL DEFAULT x") was invisible to it, incorrectly flagging an already-safe pre-existing migration (webhook_verification) as risky. Widened the match to the whole ADD COLUMN statement so DEFAULT is detected regardless of which side of NOT NULL it's on. Regenerated dev.db from the corrected migration.
… test provisions DatabaseManager's no-args constructor always built a real PostgresDatabasePool, falling back to a hardcoded postgres://postgres:postgres@localhost:5432/yieldvault connection string when DATABASE_URL was unset. No test suite runs a real Postgres at that address, so every isHealthy() check against the singleton `db` export failed for a reason entirely unrelated to what a given test was actually exercising — most visibly, /health always reported databasePrimary/databaseReplica as "down" and returned 503. Added NoopDatabasePool (resolves queries to empty results, reports healthy) and use it instead of a real Postgres pool when NODE_ENV=test and DATABASE_URL is unset — mirroring the existing fallback pattern this codebase already uses for Redis and the deposits rate limiter. Real production/dev behavior (DATABASE_URL set, or NODE_ENV=production) is unchanged.
VaultSummaryResponseSchema declared totalAssets/totalShares as z.number() and had no field for sharePrice, but the real GET /api/v1/vault/summary handler (buildVaultSummaryResponseFromDb in index.ts) returns both as Decimal-backed strings — this API's established convention for money fields, to avoid floating-point precision loss (see TransactionItemSchema. amount in the same file, already z.string()) — and always includes sharePrice. HealthResponseSchema was similarly missing lastIndexedLedger, which GET /health has always returned. Both schemas are .strict(), so every real response failed validation: missing fields as "unrecognized keys", and totalAssets/totalShares as wrong-type errors. Fixed the schemas, the fixture payloads in issues711.test.ts that encoded the same stale shape, and the inline assertions in openApiContractTests.test.ts (numeric-finite check now Number()-coerces the string fields; the additionalProperties allowlist now includes sharePrice). Regenerated the committed schema-snapshots/*.json files to match.
… summary
- index.ts had no catch-all 404 handler, so an unmatched route fell through
to Express's default plain-text 404 instead of this API's JSON error
format. Added one as the final middleware.
- buildImpersonatedVaultState() built its "summary" field from the old
buildVaultSummaryResponse() mock (always zeroed numbers) instead of the
DB-backed buildVaultSummaryResponseFromDb() the real GET
/api/v1/vault/summary route serves, so an admin impersonating a wallet
saw stale/wrong balances instead of what that wallet's own dashboard
shows. Removed the now-unused mock function.
- transactionEndpoints.ts's GET /api/v1/transactions handler called the
async buildTransactionsResponse() without awaiting it in the
no-walletAddress branch, so res.json() serialized the pending Promise —
which has no enumerable own properties — as "{}", and the DateRangeParseError
catch below it could never actually catch anything thrown inside that
promise.
…d25519 keys VALID_TEST_WALLET / SECOND_TEST_WALLET / THIRD_TEST_WALLET (and the identical MOCK_WALLET_ADDRESS in listEndpoints.ts, backing MOCK_TRANSACTIONS) were hand-typed, regex-shaped strings — correct character set, but not a real Ed25519 public key, so they failed StrKey.isValidEd25519PublicKey. Endpoints validated only via the looser @yieldvault/api-schemas regex (e.g. deposits) tolerated them; anything using the stricter check (walletAddressSchema in middleware/validate.ts — login, nonce, signed actions) rejected every test using these constants outright. Replaced all four with real keypair public keys, keeping listEndpoints.ts's copy in sync with setup.ts's since MOCK_TRANSACTIONS's wallet ownership is asserted against VALID_TEST_WALLET in several tests. fix(test-env): relax rate-limit/adaptive-throttle defaults broadly, restore true values where a test specifically exercises them setup.ts already relaxed RATE_LIMIT_ADMIN_MAX/RATE_LIMIT_WRITES_MAX for tests; extended the same treatment to RATE_LIMIT_AUTH_MAX, DEPOSITS_RATE_LIMIT_MAX, RATE_LIMIT_READS_MAX, SUMMARY_RATE_LIMIT_MAX, API_RATE_LIMIT_MAX_REQUESTS, and ADAPTIVE_THROTTLE_SCORE_THRESHOLD — any integration test file exercising real auth/read/write/deposit routes across several `it` blocks, or several validation-error responses, could exhaust these production-tight defaults or trip the adaptive-throttle block well before its assertions were about rate limiting at all. Two dedicated suites test these mechanisms at their real defaults and restore them locally (before importing the modules that read them once at load time): rateLimiter.test.ts's "depositsLimiter on vault deposit/ withdrawal routes" block and rateLimiter.auth_transfer.test.ts (both need the true DEPOSITS_RATE_LIMIT_MAX=10), and api.test.ts (has its own adaptive-throttle escalation test and already resets that middleware's state per test, so needs the true ADAPTIVE_THROTTLE_SCORE_THRESHOLD=6 restored rather than the relaxed global default).
…e in tests webhookInputValidation.test.ts sent its admin API key via a plain 'x-api-key' header on every request; validateApiKey only recognizes 'Authorization: ApiKey <key>' (as every other admin test file already does), so every request in this file was rejected with 401 regardless of what it was actually testing. rbac.test.ts registered webhooks with eventTypes: ['transaction.created'], which isn't one of the two valid event types (transaction.deposit.created, transaction.withdrawal.created — see WEBHOOK_EVENT_TYPES), so registration itself failed and later assertions dereferenced the (absent) created.body.endpoint.id.
…mock, stale regex) - adminFeatures.test.ts: the deposit test's walletAddress had lowercase letters, which VaultDepositBodySchema's regex rejects (Stellar addresses are uppercase base32); and its webhook-delivery assertion raced the outbox's background poller with a fixed 30ms wait instead of draining it explicitly via eventOutboxService.processOutbox(). - withdrawalRecoveryEndpoint.test.ts's deposit test 500'd because referralService.recordDeposit() (called on every deposit) resolves the wallet's canonical identity and opens its own Prisma transaction for referral bookkeeping, neither of which this file's minimal Prisma mock supports — and neither of which this suite (withdrawal saga recovery) is testing. Mocked referralService directly instead of expanding the Prisma mock's surface for an unrelated subsystem. - jobGovernanceMetrics.test.ts's three metric-matching regexes anchored the closing brace immediately after job_name="...", but the actual Prometheus output has an additional app="yieldvault-backend" default label (register.setDefaultLabels) before the brace closes. Widened the regexes to tolerate any additional labels.
backend-governance.yml never installed/built packages/api-schemas before running backend commands, so every step that imports @yieldvault/api-schemas (most of the backend, transitively via middleware/validate.ts) failed with "Cannot find module 'zod'" — that package's own dependency, never installed because its package.json/package-lock.json were never touched. Added the same "Build shared API schemas" step the other two workflows (Monorepo CI, Dependency Vulnerability Audit) already have. The cargo-audit 0.22.1 pin (previous commit) still failed to install: an unlocked `cargo install` re-resolves that crate's dependency tree against whatever's newest on crates.io today, and several of those transitive deps have since raised their own MSRV past the workspace's pinned rustc 1.85.0 — the exact error message suggested the fix. Added --locked to install against the dependency versions actually validated for 0.22.1's release, instead of the latest crates.io allows.
fetchCurrentApy() queried a table named "vault_metrics", but the actual raw-SQL migration (migrations/001_initial_storage.sql) creates "vault_metrics_snapshots" — every other query in this codebase referencing that table already uses the correct name (see scripts/postgres-migrations.js's drift check, which explicitly checks for "vault_metrics_snapshots"). Against a real Postgres database (e.g. the backend-governance CI job), this raised "relation vault_metrics does not exist" and crashed the entire APY snapshot job and every endpoint that depends on it, even though the query's own result is discarded (this function returns a synthetic value pending a real Soroban RPC integration).
…ed stable one The "Set up Rust nightly (cargo-fuzz)" step only installs the nightly toolchain as an available option — it doesn't change the active default, which stays pinned to rust-toolchain.toml's 1.85.0 (rustup directory overrides don't apply just because a toolchain was installed). The very next step, "Run vault share-price fuzz", already accounts for this by invoking `cargo +nightly fuzz run ...` explicitly, but "Install cargo-fuzz" ran plain `cargo install`, resolving under 1.85.0 and failing outright: cargo-fuzz 0.13.2's own locked dependencies (cargo-platform, cargo_metadata) require rustc 1.86–1.91. Added the same `+nightly` override to the install step so it builds under the toolchain it's actually meant for.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1162