feat: add healthchecks to Dockerfiles for api, basket, links, and uptime services#380
Conversation
|
@sekhar08 is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Greptile SummaryThis PR adds Docker native health checking to all four service Dockerfiles (
Confidence Score: 4/5Safe to merge — all health probe targets exist, ports are correct, and the base-image switch is justified; only minor style issues remain. The changes are additive and well-scoped. Health endpoints are verified to exist in all four services. The only functional concern (unhandled fetch rejection) still results in the correct unhealthy exit code in Bun, making it a style issue rather than a runtime bug. Missing newlines and unconventional ENTRYPOINT ordering are cosmetic. No logic errors or security regressions introduced. healthcheck.ts — should add try/catch for explicit error handling. Important Files Changed
Sequence DiagramsequenceDiagram
participant Docker as Docker Daemon
participant HC as healthcheck.ts (bun)
participant Svc as Service (/health)
Note over Docker: Every 30s (after 10s start-period)
Docker->>HC: exec ["bun", "/app/healthcheck.ts"]
HC->>Svc: GET http://localhost:{HEALTHCHECK_PORT}/health
alt HTTP 2xx
Svc-->>HC: 200 { status: "ok" }
HC->>Docker: exit 0 (healthy)
else HTTP non-2xx
Svc-->>HC: 5xx / 4xx
HC->>Docker: exit 1 (unhealthy)
else Connection refused / timeout
HC->>Docker: unhandled rejection → exit 1 (unhealthy)
end
Note over Docker: After 3 retries unhealthy → container marked unhealthy
Reviews (1): Last reviewed commit: "feat: add healthchecks to Dockerfiles fo..." | Re-trigger Greptile |
| const r = await fetch(`http://localhost:${port}/health`); | ||
| process.exit(r.ok ? 0 : 1); |
There was a problem hiding this comment.
Missing error handling on fetch
fetch() can throw a network-level error (e.g., ECONNREFUSED) if the service is not yet listening. In Bun, an unhandled promise rejection does cause a non-zero exit code so the healthcheck will still be marked unhealthy — but this relies on runtime implementation details rather than explicit intent, and the error won't be surfaced in Docker logs.
A try/catch makes the failure mode explicit:
| const r = await fetch(`http://localhost:${port}/health`); | |
| process.exit(r.ok ? 0 : 1); | |
| try { | |
| const r = await fetch(`http://localhost:${port}/health`); | |
| process.exit(r.ok ? 0 : 1); | |
| } catch { | |
| process.exit(1); | |
| } |
| HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \ | ||
| CMD ["bun", "/app/healthcheck.ts"] | ||
|
|
||
| ENTRYPOINT [] | ||
| CMD ["./server"] |
There was a problem hiding this comment.
ENTRYPOINT [] placed after HEALTHCHECK
Convention is to declare ENTRYPOINT before HEALTHCHECK (and CMD) so the intended runtime contract is clear before the health probe is defined. Docker processes these as a final metadata set so this doesn't break anything, but the ordering is slightly unexpected for readers.
The same ordering applies to links.Dockerfile and uptime.Dockerfile.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
api.Dockerfile
Outdated
| HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \ | ||
| CMD ["bun", "/app/healthcheck.ts"] | ||
|
|
||
| CMD ["bun", "run", "src/index.ts"] No newline at end of file |
|
Healthcheck concept is solid — all services have A few things to address:
Please retarget to |
98c780c to
ba1427e
Compare
|
|
● ## Description
Add production-ready
HEALTHCHECKinstructions to all four service Dockerfiles, enabling Docker to natively monitor container health.Changes:
healthcheck.tsscript viabun(exec form)gcr.io/distroless/cctooven/bun:1.3.4-distroless(preserves distroless security posture), addedENTRYPOINT []to preventbun from interpreting the compiled binary as JS, added HEALTHCHECK
gcr.io/distroless/basetooven/bun:1.3.4-distroless, port 2500gcr.io/distroless/basetooven/bun:1.3.4-distroless, port 4000/healthusingHEALTHCHECK_PORTenv var, used by all 4 servicesAll health checks use exec form (
CMD ["bun", "/app/healthcheck.ts"]) which works in distroless without a shell. Build stage versions are untouched.HEALTHCHECK config:
--interval=30s --timeout=3s --start-period=10s --retries=3Checklist