fix: make aggregate health reflect dependencies - #1784
Merged
Conversation
📝 WalkthroughWalkthrough등록된 health check가 집계 Changes집계 Health 엔드포인트
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CrocoApp
participant HealthCheckRegistry
participant sanitizeHealthCheckResult
Client->>CrocoApp: GET /health
CrocoApp->>HealthCheckRegistry: 등록된 health check 실행
HealthCheckRegistry-->>CrocoApp: health aggregate 결과
CrocoApp->>sanitizeHealthCheckResult: 결과 sanitization
sanitizeHealthCheckResult-->>CrocoApp: 제한 및 마스킹된 결과
CrocoApp-->>Client: 200 또는 503 응답
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.changeset/aggregate-http-health.md:
- Line 2: Update the changeset entry for `@croco/transports-http` in
aggregate-http-health.md from patch to minor to reflect the breaking public HTTP
contract changes while the package remains on the 0.x version line.
🪄 Autofix
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 Plus
Run ID: a777813a-c665-419c-af43-7e5655abe9ba
📒 Files selected for processing (7)
.changeset/aggregate-http-health.mddocs/troubleshooting/diagnostics.mdpackages/transports-http/README.mdpackages/transports-http/src/libs/CrocoApp.tspackages/transports-http/src/libs/operationalEndpoints.tspackages/transports-http/src/tests/HealthCheck.spec.tspackages/transports-http/src/tests/OperationalEndpoints.spec.ts
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-08-09T09:32:25.515Z · Commit: 8719c22 |
Member
Author
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/transports-http/src/libs/operationalEndpoints.ts (2)
254-282: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift전체 순회 비용을 별도로 제한하십시오.
Line [254-282]의collectionLimit은 각 컬렉션의 출력 항목 수만 제한합니다. 재귀 호출마다 같은 한도가 다시 적용되므로 각 레벨에 50개 항목이 있는 깊이 5 구조는 약50^5개의 leaf 경로를 방문할 수 있습니다.Object.entries도slice전에 모든 object key를 materialize합니다. 큰 또는 중첩된details가 반환되면/health요청이 CPU와 메모리를 과도하게 사용하여 응답 지연 또는 프로세스 자원 고갈을 일으킬 수 있습니다. 전체 재귀에 공유되는 node/byte budget을 추가하고, 객체는 한도에 도달하면 즉시 순회를 중단하십시오. Flat object와 공유 하위 객체를 사용하는 회귀 테스트도 추가하십시오.🤖 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/operationalEndpoints.ts` around lines 254 - 282, Update redactValue to enforce a shared node/byte budget across the entire recursive traversal, rather than resetting collectionLimit at each level. Avoid materializing all object keys with Object.entries before limiting; iterate entries incrementally and stop immediately when the shared budget is exhausted, returning the existing truncation marker as appropriate. Preserve redaction behavior while ensuring both flat large objects and shared nested-object graphs are bounded, and add regression tests covering those cases.
212-213: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winhealth 상세 문자열 한도의 상한을 적용하십시오.
path: packages/transports-http/src/libs/operationalEndpoints.ts:212-213에서 health 상세 문자열 제한이messageLimit에 전적으로 의존합니다.messageLimit은 100자가 넘어 설정할 수 있으므로, health 상세에는 health 전용 상한을 사용하십시오.🤖 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/operationalEndpoints.ts` around lines 212 - 213, Update the health detail configuration near stringLimit and collectionLimit so stringLimit uses a health-specific maximum rather than relying solely on messageLimit. Clamp messageLimit to the established health detail string-limit constant, preserving the existing collectionLimit behavior.
🤖 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.
Outside diff comments:
In `@packages/transports-http/src/libs/operationalEndpoints.ts`:
- Around line 254-282: Update redactValue to enforce a shared node/byte budget
across the entire recursive traversal, rather than resetting collectionLimit at
each level. Avoid materializing all object keys with Object.entries before
limiting; iterate entries incrementally and stop immediately when the shared
budget is exhausted, returning the existing truncation marker as appropriate.
Preserve redaction behavior while ensuring both flat large objects and shared
nested-object graphs are bounded, and add regression tests covering those cases.
- Around line 212-213: Update the health detail configuration near stringLimit
and collectionLimit so stringLimit uses a health-specific maximum rather than
relying solely on messageLimit. Clamp messageLimit to the established health
detail string-limit constant, preserving the existing collectionLimit behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eac958c6-f207-44f4-bfca-d7fb9d23ad66
📒 Files selected for processing (3)
.changeset/aggregate-http-health.mdpackages/transports-http/src/libs/operationalEndpoints.tsscripts/create-croco-app-generated-smoke.mts
kang-heewon
force-pushed
the
fix/1697-aggregate-health
branch
2 times, most recently
from
August 9, 2026 06:47
db17e43 to
dae84ca
Compare
kang-heewon
force-pushed
the
fix/1697-aggregate-health
branch
from
August 9, 2026 09:22
dae84ca to
4a34364
Compare
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.
Outcome
/healthnow evaluates checks registered throughHealthCheckRegistry.register()and returns the sanitized aggregate health contract. Failed, thrown, or timed-out dependency checks producestatus: downwith HTTP 503, while/health/liveremains a lightweight dependency-independent process probe.Health and readiness details share explicit response bounds: strings are capped at 100 characters, each object or array at 50 entries, and each check at 500 traversed nodes and 10,000 characters. Sensitive keys are redacted,
stack/causeand non-JSON callable values are omitted, and serialization hooks cannot bypass those bounds.Fixes #1697
Verification
pnpm problem-registry:check— 597/597 discoveries passedpnpm --filter @croco/transports-http test— 33 files, 435 tests passedpnpm --filter @croco/transports-http typecheck— passedpnpm --filter @croco/transports-http lint— passed with 0 warnings/errorspnpm strict-contract-typecheck— 503 baseline diagnostics matchedReview gates
Residual risk
Consumers that treated
/healthas unconditional liveness must use/health/live; this intentional compatibility change is documented and receives a minor release on the package 0.x version line.