feat: implement production-grade liveness and readiness probes - #4292
feat: implement production-grade liveness and readiness probes#4292ujjwalredd wants to merge 4 commits into
Conversation
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| res.status(503).json({ | ||
| status: "error", | ||
| message: "Service is not ready", | ||
| detail: error instanceof Error ? error.message : "Unknown error" |
There was a problem hiding this comment.
P2: Readiness probe exposes internal error details in HTTP response
Internal error messages are leaked in the readiness probe 503 response body.
Remove the detail field from the 503 response; log the full error server-side only.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="apps/api/src/controllers/v0/readiness.ts">
<violation number="1" location="apps/api/src/controllers/v0/readiness.ts:37">
<priority>P2</priority>
<title>Readiness probe exposes internal error details in HTTP response</title>
<evidence>The catch block returns the raw exception message to unauthenticated callers: `detail: error instanceof Error ? error.message : "Unknown error"`. This leaks internal failure details such as database connection errors, Redis failure messages, or any downstream exception text to anyone who can reach the health endpoint.</evidence>
<recommendation>Remove the `detail` field from the 503 JSON response, or replace it with a static opaque message such as `detail: "Readiness check failed"`. Continue logging the full error server-side via the existing `logger.error` call.</recommendation>
</violation>
</file>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/controllers/v0/liveness.ts">
<violation number="1" location="apps/api/src/controllers/v0/liveness.ts:10">
P2: When Redis stops replying while its socket remains `ready`, this race returns 503 but leaves the ioredis `ping()` pending; successful pings also leave the two-second timer alive until it fires. Use a command-level timeout or coalesced probe, and clear the timer in `finally`, so repeated health checks cannot accumulate work on the shared client.</violation>
</file>
<file name="apps/api/src/controllers/v0/readiness.ts">
<violation number="1" location="apps/api/src/controllers/v0/readiness.ts:10">
P2: When a Redis or PostgreSQL check hangs, `withTimeout` only rejects the wrapper; the underlying operation continues after the handler returns 503. Repeated readiness probes can accumulate pending Redis commands or occupy PostgreSQL pool clients, so use cancellable/native client timeouts for these checks and clean up the race timer.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| return res.status(503).json({ status: "error", message: "Redis connection is not ready" }); | ||
| } | ||
| // Set a short timeout to prevent the ping from hanging indefinitely | ||
| await Promise.race([ |
There was a problem hiding this comment.
P2: When Redis stops replying while its socket remains ready, this race returns 503 but leaves the ioredis ping() pending; successful pings also leave the two-second timer alive until it fires. Use a command-level timeout or coalesced probe, and clear the timer in finally, so repeated health checks cannot accumulate work on the shared client.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/controllers/v0/liveness.ts, line 10:
<comment>When Redis stops replying while its socket remains `ready`, this race returns 503 but leaves the ioredis `ping()` pending; successful pings also leave the two-second timer alive until it fires. Use a command-level timeout or coalesced probe, and clear the timer in `finally`, so repeated health checks cannot accumulate work on the shared client.</comment>
<file context>
@@ -2,8 +2,17 @@ import { Request, Response } from "express";
+ return res.status(503).json({ status: "error", message: "Redis connection is not ready" });
+ }
+ // Set a short timeout to prevent the ping from hanging indefinitely
+ await Promise.race([
+ redisRateLimitClient.ping(),
+ new Promise((_, reject) => setTimeout(() => reject(new Error("Redis ping timed out")), 2000))
</file context>
| import { config } from "../../config"; | ||
|
|
||
| const withTimeout = <T>(promise: Promise<T>, timeoutMs: number, name: string): Promise<T> => { | ||
| return Promise.race([ |
There was a problem hiding this comment.
P2: When a Redis or PostgreSQL check hangs, withTimeout only rejects the wrapper; the underlying operation continues after the handler returns 503. Repeated readiness probes can accumulate pending Redis commands or occupy PostgreSQL pool clients, so use cancellable/native client timeouts for these checks and clean up the race timer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/controllers/v0/readiness.ts, line 10:
<comment>When a Redis or PostgreSQL check hangs, `withTimeout` only rejects the wrapper; the underlying operation continues after the handler returns 503. Repeated readiness probes can accumulate pending Redis commands or occupy PostgreSQL pool clients, so use cancellable/native client timeouts for these checks and clean up the race timer.</comment>
<file context>
@@ -4,28 +4,36 @@ import { db, dbRr } from "../../db/connection";
+import { config } from "../../config";
+
+const withTimeout = <T>(promise: Promise<T>, timeoutMs: number, name: string): Promise<T> => {
+ return Promise.race([
+ promise,
+ new Promise<T>((_, reject) => setTimeout(() => reject(new Error(`${name} check timed out after ${timeoutMs}ms`)), timeoutMs))
</file context>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
1 participant
Resolves #4291
This PR implements the feature requested in the associated issue.
It implements production-grade readiness and liveness probes in
src/controllers/v0/readiness.tsandsrc/controllers/v0/liveness.tsthat actively verify connections to Postgres and Redis before returning200 OK. This resolves existing TODOs in those files and ensures that orchestration systems like Kubernetes only route traffic to fully healthy pods.Summary by cubic
Adds production-grade liveness and readiness probes so orchestrators route traffic only to healthy pods. Previously both endpoints returned 200 without checks; now liveness verifies the rate limiter Redis is responsive, and readiness verifies rate limiter Redis, queue Redis, and optionally Postgres before advertising readiness.
Liveness returns 503 if the rate limiter Redis client is not ready or its PING exceeds 2s; timers are cleared in finally to prevent leaks. Readiness requires both Redis clients to be ready and respond within 3s; when
config.USE_DB_AUTHENTICATIONis true, it runs SELECT 1 on main and replica viadrizzle-orm. On failure, it logs the error and returns 503 with "Service is not ready" and no internal details.Written for commit 3985229. Summary will update on new commits.