fix(web): add dedicated health route to prevent memory leak (#2344)#2582
fix(web): add dedicated health route to prevent memory leak (#2344)#2582themillhauz wants to merge 2 commits intokarakeep-app:mainfrom
Conversation
The /api/health endpoint was handled by the Hono catch-all route in apps/web/app/api/[[...route]]/route.ts, which runs the nextAuth middleware on every request. This middleware calls createContextFromRequest() which calls next-auth init(), allocating ~14 closure objects per request wrapping all DrizzleAdapter methods. With healthchecks running every 30-60 seconds (Docker, Uptime Kuma, etc.), these objects accumulate in the V8 old generation heap faster than GC can collect them, causing gradual memory growth of ~3-17 MB/hour depending on healthcheck interval. This is the root cause of the memory leak reported in issue karakeep-app#2344. Fix: add a dedicated Next.js route at apps/web/app/api/health/route.ts. Next.js static routes take priority over [[...route]] catch-all routes, so healthcheck requests no longer trigger auth context initialization. The response format is preserved: {status: 'ok', message: '...'} Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WalkthroughAdds a new unauthenticated health check API route at Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 a dedicated Next.js route at
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 9f3db0e |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/app/api/health/route.ts (1)
4-7: Make the health route explicitly dynamic.This handler is deterministic, so it is worth pinning it with
export const dynamic = "force-dynamic"to keep/api/healthfrom ever being prerendered or served as a cached"ok"response if Cache Components or routing defaults change. That is an inference from the current Next.js route-handler caching behavior. (nextjs.org)♻️ Suggested change
export const runtime = "nodejs"; +export const dynamic = "force-dynamic"; export function GET() { return Response.json({ status: "ok", message: "Web app is working" }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/health/route.ts` around lines 4 - 7, Add an explicit dynamic export to prevent prerendering: in the route.ts where runtime is exported and GET is defined (export const runtime = "nodejs"; and function GET()), add export const dynamic = "force-dynamic" so the /api/health handler is always dynamic and never cached/prerendered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/app/api/health/route.ts`:
- Around line 4-7: Add an explicit dynamic export to prevent prerendering: in
the route.ts where runtime is exported and GET is defined (export const runtime
= "nodejs"; and function GET()), add export const dynamic = "force-dynamic" so
the /api/health handler is always dynamic and never cached/prerendered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96f7601a-7200-4754-9994-3e5d9fbcc26c
📒 Files selected for processing (1)
apps/web/app/api/health/route.ts
Per CodeRabbit review: without explicit dynamic export, Next.js may prerender or cache the /api/health response as a static page. Adding export const dynamic = 'force-dynamic' ensures the handler is always evaluated at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
There's already a dedicated health route. And the leak is usually caused by people configuring the healthcheck against the homepage. So, we'll be fixing the leak in the homepage instead. Thanks for the PR though. |
fixes #2344
Problem
The
/api/healthendpoint was handled by the Hono catch-all route inapps/web/app/api/[[...route]]/route.ts, which runs thenextAuthmiddleware on every request. This middleware calls
createContextFromRequest()→next-auth init(), which allocates ~14new closure objects wrapping all DrizzleAdapter methods per request.
With healthchecks running every 30–60 seconds (Docker built-in, Uptime
Kuma, Pangolin, Blackbox Exporter, etc.), these objects accumulate in the
V8 old generation heap faster than GC can collect them, causing gradual
memory growth of ~3–17 MB/hour depending on healthcheck interval.
This is the root cause of the memory leak reported in #2344. Several
users confirmed that disabling healthchecks stopped the memory growth.
Fix
Add a dedicated Next.js route at
apps/web/app/api/health/route.ts.Next.js static routes take priority over
[[...route]]catch-all routes,so healthcheck requests no longer trigger auth context initialization.
The response format is preserved:
{"status":"ok","message":"..."}Testing
Verified locally:
GET /api/healthreturns200 OKwithout anyauth/session calls appearing in logs.