feat: add /readyz readiness probe with database check#113
Conversation
|
On it, @rubenhensen! I'm reviewing your readiness probe PR now and will post a consolidated review shortly. |
|
/dobby review |
|
On it! I'm reviewing the pull request in postguard-business#113 for correctness and rule-compliance, and will post a consolidated review shortly. |
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the /readyz readiness probe (Closes #97). The implementation is correct: select 1 via drizzle, 200 {status:ready,checks:{database:ok}} on success and 503 {status:unavailable,checks:{database:down}} on failure, distinct from the /health liveness check. Title is conventional-commit, the Closes #97 keyword is present, and the deferred IRMA/startup-validation scope is called out explicitly. Rule checks (SvelteKit route exports, CSP, promised-vs-delivered, PR title, close-keyword, vitest hoisting) all pass. Two non-blocking nits inline — no changes required to merge.
|
Reviewed! I left a COMMENT review with two non-blocking nits:
Everything else looks good — no blocking issues. Good to merge when you're ready. |
|
/dobby fix the nits |
|
On it! I'll fix the nitpick comments and open a draft PR with the changes, then run through a review loop to make sure everything's clean before marking it ready. |
Addresses review nits on #113: - Mark both the 200/ready and 503/unavailable responses Cache-Control: no-store so an intermediary cache can't serve a stale readiness verdict. - Add tests/unit/readyz-endpoint.test.ts covering both branches by stubbing db.execute, locking in the 503/down shape that the e2e test can't reach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Rules Dobby 2 — clean. Per-rule compliance sweep + Review Dobby 2's pass both came back with zero blocking findings.
src/routes/readyz/+server.tsexports onlyGET(theNO_STOREobject is a plainconst, not exported) → no SvelteKit adapter build-time export violation.- Unit test uses the correct
vi.hoisted()→vi.mock()ordering; 85 unit tests pass (2 new readyz) and svelte-check reports 0 errors. - Both success (200/ready) and failure (503/unavailable) paths are covered by unit tests, plus a Playwright e2e happy-path test — satisfies the tests-required-on-fixes rule.
no-storeon both 200 and 503 correctly mirrors/health; no sensitive data leaked (caught error is logged server-side only).
Approving.
|
Reviewed the nit-fix diff — all rules pass. ✅
Approved — good to merge when you're ready. |
1 participant
Closes #97.
Adds a
/readyzreadiness probe, distinct from the existing/healthliveness check.What & why
/healthreturns{ status: 'ok' }unconditionally — it says the process is up, not that it can serve traffic./readyzruns aselect 1against Postgres and returns:{ status: 'ready', checks: { database: 'ok' } }when the DB is reachable;{ status: 'unavailable', checks: { database: 'down' } }when it isn't.This lets an orchestrator hold traffic when the database is unreachable. The container liveness
HEALTHCHECKintentionally stays on/health; readiness gates should point at/readyz.Covered by an e2e test (
tests/e2e/readyz.e2e.ts) that mirrors the existinghealth.e2e.ts— in CI the Postgres service is up, so it asserts the 200/ready path.Scope notes