Skip to content

feat: RPC timeout budgets for policy deploy + circuit breaker for verification-service - #381

Merged
davedumto merged 2 commits into
Vellar-Wallet:devfrom
onuibeblessing2019-hash:fix/326-327-timeout-and-circuit-breaker
Aug 30, 2026
Merged

feat: RPC timeout budgets for policy deploy + circuit breaker for verification-service#381
davedumto merged 2 commits into
Vellar-Wallet:devfrom
onuibeblessing2019-hash:fix/326-327-timeout-and-circuit-breaker

Conversation

@onuibeblessing2019-hash

Copy link
Copy Markdown

Closes #327
Closes #326

#327 — timeout budgets for policy deployment RPC calls

services/policy-service/src/deploy.ts's deploy path (getAccount, simulateTransaction, prepareTransaction, sendTransaction, the getTransaction polling loop) had no explicit timeout on any individual RPC call — only the polling loop had an overall deadline.

Real finding while implementing this: rpc.Server's constructor accepts a timeout option in @stellar/stellar-sdk's TypeScript types, but as of SDK 16.0.1 it has no effectRpcServer's constructor calls createHttpClient(opts.headers), which only ever receives headers, never opts.timeout. Confirmed by reading rpc/axios.js/http-client/fetch-client.js, and experimentally: a test against a deliberately-hanging local server using the SDK's own timeout option hung indefinitely. Implemented a real, self-contained timeout (withTimeoutError, a Promise.race against this module's own timer) instead of relying on the non-functional SDK option — documented prominently in the code so it isn't "fixed" back to the broken approach later.

  • New env vars: DEPLOY_RPC_TIMEOUT_MS (default 10s, bounds each individual RPC call) and DEPLOY_POLL_TIMEOUT_MS (default 60s, bounds the overall polling loop) — documented in policy-service/README.md per the issue's requirement.
  • New distinct error codes: deploy_rpc_timeout (a single call stalled) vs. the existing deploy_timeout (poll loop exceeded its overall budget) vs. every other existing failure code — a timeout is never confused with a real RPC/contract failure.
  • 7 new tests in deploy.test.ts, using a real local HTTP server that never responds (the only reliable way to exercise a genuine timeout end-to-end) — confirms the budget is actually enforced, is configurable, and is distinct from a real connection-refused error.

#326 — circuit breaker for api-gateway → verification-service

No existing circuit-breaker dependency in this repo, so added a small, dependency-free state machine (packages/service-kit/src/circuit-breaker.ts) — closed → open (after failureThreshold consecutive failures) → half-open (one trial call after cooldownMs) → closed or back to open, matching the issue's exact requirement.

  • Wired into api-gateway's /verification/* proxy via @fastify/http-proxy's preHandler (fast-fail 503 while open, no network attempt) and replyOptions.onResponse/onError hooks (record success/failure) — only a genuine connection-level failure counts against the breaker; a normal upstream 4xx/5xx (the verification service correctly reporting a domain outcome) does not.
  • New env vars: VERIFICATION_CB_FAILURE_THRESHOLD (default 5), VERIFICATION_CB_COOLDOWN_MS (default 30s) — documented in api-gateway/README.md.
  • New Prometheus counter vela_circuit_breaker_state_changes_total{breaker="verification-service"} per the issue's "add a metric tracking circuit breaker state changes" requirement, plus a log line on every transition.
  • 17 new unit tests in circuit-breaker.test.ts (closed/open/half-open transitions, the metric hook, the lower-level beforeCall/recordOutcome pair the gateway integration actually uses since @fastify/http-proxy can't wrap a promise this code controls) + 2 new integration tests in api-gateway/server.test.ts exercising the full cycle through a real gateway instance against a real (unlistened-port) connection failure, confirming: opens after 2 consecutive failures, fast-fails with 503 + retryAfterMs, recovers via a half-open trial once the real upstream comes back, and — separately — confirms a real reachable upstream returning 404 five times in a row never trips a breaker configured to open on just 1 failure.

Test plan

  • packages/service-kit: 71/71 tests pass (circuit-breaker.test.ts new, everything else unaffected).
  • services/policy-service: 50/50 tests pass (deploy.test.ts new).
  • services/api-gateway: 15/15 tests pass (2 new circuit-breaker integration tests).
  • npx tsc --noEmit clean in all 3 touched packages.

@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@onuibeblessing2019-hash 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! 🚀

Learn more about application limits

@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

@onuibeblessing2019-hash is attempting to deploy a commit to the david's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
github-actions Bot changed the base branch from main to dev August 30, 2026 11:44
@github-actions

Copy link
Copy Markdown

Hi @onuibeblessing2019-hash — thanks for the contribution!

We do not accept pull requests to the main branch. All contributions go to the dev branch, so I've automatically retargeted this PR from main to dev for you.

You don't need to reopen anything — your work is preserved and this PR now targets dev. Going forward, please set the base branch to dev when you open a PR. 🙏

…and-circuit-breaker

# Conflicts:
#	packages/service-kit/src/index.ts
#	packages/service-kit/src/metrics.ts
#	services/api-gateway/README.md
#	services/api-gateway/src/server.ts
@onuibeblessing2019-hash

Copy link
Copy Markdown
Author

Merged upstream/dev in to resolve a conflict (this repo's base branch is dev, not main, despite main being the GitHub-configured default — same as the sibling vellar-sdk repo's contribution workflow).

Two real, pre-existing bugs surfaced during that merge, worth flagging separately from #326/#327's own scope:

  1. Fixed in this PR (a hard blocker — the whole @vellar/service-kit package throws ReferenceError: workerProcessingLagSeconds is not defined at import time on current dev): packages/service-kit/src/metrics.ts's domainMetrics object references workerProcessingLagSeconds, but no such const was ever defined anywhere in the file — confirmed by checking upstream/dev directly (not just my merge). Restored it as a Gauge matching its sibling workerQueueDepth's exact shape, per what metrics.test.ts's own existing (previously-unreachable) assertions expect.
  2. Not fixed, disclosed onlyservices/policy-service has 67 real TypeScript errors and 28 failing tests on clean upstream/dev (confirmed via an isolated worktree, byte-identical error set before and after my merge — zero new errors from this PR). Root cause looks like an incomplete/bad merge already on dev: server.test.ts has literal duplicate top-of-file imports, and server.ts references several names (validateDefinition, deploymentDeps, verifyAndRecordAttach, etc.) that aren't imported or defined anywhere in the file. This is unrelated to [hard] Add circuit breaker for api-gateway calls to verification-service #326/[medium] Add timeout budgets for policy deployment RPC calls #327 and out of scope for this PR to fix — flagging so it isn't mistaken for something this PR broke.

@davedumto
davedumto merged commit a0d1a48 into Vellar-Wallet:dev Aug 30, 2026
1 check failed
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.

[medium] Add timeout budgets for policy deployment RPC calls [hard] Add circuit breaker for api-gateway calls to verification-service

2 participants