fix: make HTTP readiness use readiness indicators - #1361
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughHealth-core now aggregates readiness indicators independently, the HTTP registry exposes readiness registration and checks, and readiness routes return sanitized detailed results. Tests and documentation cover isolation, failures, timeouts, redaction, aliases, and empty readiness registrations. ChangesReadiness HTTP contract
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CrocoApp
participant HealthCheckRegistry
participant HealthCheckService
Client->>CrocoApp: GET /ready or /health/ready
CrocoApp->>HealthCheckRegistry: checkReadiness()
HealthCheckRegistry->>HealthCheckService: checkReadiness()
HealthCheckService-->>HealthCheckRegistry: Detailed readiness result
HealthCheckRegistry-->>CrocoApp: Registry readiness result
CrocoApp->>CrocoApp: Sanitize details
CrocoApp-->>Client: HTTP 200 or 503 response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-11T19:24:25.231Z · Commit: 620efcb |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/transports-http/src/libs/CrocoApp.ts (1)
405-419: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
/health/ready와/ready핸들러 로직 중복 - 공통 함수로 추출 권장.두 라우트가
checkReadiness()+sanitizeReadinessResult()+ 상태 코드 결정 로직을 완전히 동일하게 반복하고 있습니다. 두 alias가 항상 동일하게 동작해야 한다는 PR 목표를 고려하면, 공통 핸들러로 추출해 향후 한쪽만 변경되어 어긋나는 위험을 줄이는 것이 좋습니다.♻️ 중복 제거 제안
+ const readinessHandler = async (c: Context) => { + const result = sanitizeReadinessResult(await this.healthCheckRegistry.checkReadiness()); + return c.json(result, result.status === "up" ? 200 : 503); + }; + - this.hono.get("/health/ready", async (c) => { - const result = sanitizeReadinessResult(await this.healthCheckRegistry.checkReadiness()); - return c.json(result, result.status === "up" ? 200 : 503); - }); - - this.hono.get("/ready", async (c) => { - const result = sanitizeReadinessResult(await this.healthCheckRegistry.checkReadiness()); - return c.json(result, result.status === "up" ? 200 : 503); - }); + this.hono.get("/health/ready", readinessHandler); + this.hono.get("/ready", readinessHandler);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/transports-http/src/libs/CrocoApp.ts` around lines 405 - 419, Extract the duplicated readiness logic from registerSystemRoutes into a shared handler that performs checkReadiness, sanitizeReadinessResult, and status-code selection, then register both /health/ready and /ready with that handler so the aliases remain behaviorally identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/health-core/README.md`:
- Around line 235-242: Update the README integration example to use the
readiness-capable indicator classes already defined in the document, or add
complete definitions for DatabaseReadinessIndicator and RedisReadinessIndicator
before they are instantiated. Ensure the example is self-contained and every
referenced class implements the readiness contract expected by
healthService.registerReadiness.
In `@packages/health-core/src/tests/HealthCheckService.spec.ts`:
- Around line 421-454: Restore real timers after the fake-timer test by adding
an afterEach cleanup that calls vi.useRealTimers(). Scope the cleanup
appropriately for the tests in HealthCheckService.spec.ts so subsequent tests
are unaffected.
---
Outside diff comments:
In `@packages/transports-http/src/libs/CrocoApp.ts`:
- Around line 405-419: Extract the duplicated readiness logic from
registerSystemRoutes into a shared handler that performs checkReadiness,
sanitizeReadinessResult, and status-code selection, then register both
/health/ready and /ready with that handler so the aliases remain behaviorally
identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b037039-c26f-46e4-a219-e8741c92c883
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (18)
.changeset/readiness-indicator-http.mddocs/problem-code-registry.jsondocs/troubleshooting/diagnostics.mdpackages/docs/src/content/docs/api/health-core/src/classes/HealthCheckService.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/api/transports-http/src/classes/HealthCheckRegistry.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/health-core/README.mdpackages/health-core/src/libs/HealthCheckService.tspackages/health-core/src/tests/HealthCheckService.spec.tspackages/transports-http/README.mdpackages/transports-http/src/libs/CrocoApp.tspackages/transports-http/src/libs/HealthCheckRegistry.tspackages/transports-http/src/libs/operationalEndpoints.tspackages/transports-http/src/tests/HealthCheck.spec.tspackages/transports-http/src/tests/OperationalEndpoints.spec.tsscripts/static-misuse-empty-catch-allowlist.jsontsconfig/contract-strict.baseline.json
|
Addressed the out-of-diff alias-handler request in db275e1: /health/ready and /ready now share one typed readiness handler, keeping aggregation, sanitization, and status selection identical. Focused HTTP tests pass 38/38; full pre-push test and typecheck gates pass 225/225 and 224/224 tasks. |
Fixes #1312.
Summary
/readyand/health/readyexecute explicitly registered readiness indicators and map the aggregate result to 200 or 503.Verification
pnpm check— passed.pnpm typecheck— 224/224 tasks passed.pnpm test— 225/225 tasks passed.git diff --check— passed.Self-review gates
Review and QA
Migration
Applications that previously used generic
register()calls as implicit readiness inputs must register those indicators explicitly withregisterReadiness().Residual risk
Readiness and generic health registration are intentionally independent. Existing applications receive an empty, healthy readiness aggregate until they opt indicators into the readiness registry.
Summary by CodeRabbit
새로운 기능
/ready와/health/ready가 readiness 전용 체크만 실행합니다.버그 수정
문서