From c022dbc4a89f4c0ef8208fbea7f7184b24f018bf Mon Sep 17 00:00:00 2001 From: adevwithpurpose Date: Sat, 15 Aug 2026 19:13:19 -0300 Subject: [PATCH] fix(docker): use lightweight /healthz for container lifecycle healthcheck instead of the heavy monitoring route (#10311) --- .../10311-healthcheck-lifecycle-default.md | 1 + docs/guides/DOCKER_GUIDE.md | 6 ++- docs/i18n/pl/docs/guides/DOCKER_GUIDE.md | 6 ++- scripts/dev/healthcheck.mjs | 48 ++++++++++++++++--- tests/unit/docker-healthcheck-3151.test.ts | 2 +- .../unit/docker-healthcheck-base-path.test.ts | 27 +++++++++-- ...0311-healthcheck-lifecycle-default.test.ts | 12 +++++ 7 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 changelog.d/fixes/10311-healthcheck-lifecycle-default.md create mode 100644 tests/unit/probe-10311-healthcheck-lifecycle-default.test.ts diff --git a/changelog.d/fixes/10311-healthcheck-lifecycle-default.md b/changelog.d/fixes/10311-healthcheck-lifecycle-default.md new file mode 100644 index 0000000000..8a27b45d5d --- /dev/null +++ b/changelog.d/fixes/10311-healthcheck-lifecycle-default.md @@ -0,0 +1 @@ +- **fix(ops):** Docker HEALTHCHECK defaults to the lightweight `/healthz` lifecycle probe instead of the heavy `/api/monitoring/health` path, with an `OMNIROUTE_HEALTHCHECK_PATH` opt-in override ([#10311](https://github.com/diegosouzapw/OmniRoute/pull/10311)) \ No newline at end of file diff --git a/docs/guides/DOCKER_GUIDE.md b/docs/guides/DOCKER_GUIDE.md index 6740a1958e..785142b0e8 100644 --- a/docs/guides/DOCKER_GUIDE.md +++ b/docs/guides/DOCKER_GUIDE.md @@ -269,8 +269,10 @@ prefix). Traefik should route `PathPrefix(`/omniroute`)` to the container withou `StripPrefix`, so Next.js receives `/omniroute/...` and serves assets from `/omniroute/_next/...`. -The Docker healthcheck probes `/api/monitoring/health` prefixed with the active -`OMNIROUTE_BASE_PATH`. +The Docker healthcheck probes the lightweight `/healthz` lifecycle endpoint prefixed +with the active `OMNIROUTE_BASE_PATH`. `/api/monitoring/health` remains available for +human/dashboard diagnostics; to point the container HEALTHCHECK back at it (for example +for deep health enforcement), set `OMNIROUTE_HEALTHCHECK_PATH=/api/monitoring/health`. ## Docker Compose with Caddy (HTTPS Auto-TLS) diff --git a/docs/i18n/pl/docs/guides/DOCKER_GUIDE.md b/docs/i18n/pl/docs/guides/DOCKER_GUIDE.md index 662348fed5..50515eb837 100644 --- a/docs/i18n/pl/docs/guides/DOCKER_GUIDE.md +++ b/docs/i18n/pl/docs/guides/DOCKER_GUIDE.md @@ -224,8 +224,10 @@ prefiksu). Traefik powinien routować `PathPrefix(`/omniroute`)` do kontenera be `StripPrefix`, żeby Next.js otrzymywał `/omniroute/...` i serwował assety z `/omniroute/_next/...`. -Healthcheck Dockera sonduje `/api/monitoring/health` z prefiksem aktywnego -`OMNIROUTE_BASE_PATH`. +Healthcheck Dockera sonduje lekki endpoint cyklu życia `/healthz` z prefiksem aktywnego +`OMNIROUTE_BASE_PATH`. `/api/monitoring/health` pozostaje dostępny do diagnostyki +człowieka/pulpit; aby ustawić HEALTHCHECK kontenera z powrotem na niego (np. dla +głębokiej kontroli stanu), ustaw `OMNIROUTE_HEALTHCHECK_PATH=/api/monitoring/health`. ## Docker Compose z Caddy (HTTPS Auto-TLS) diff --git a/scripts/dev/healthcheck.mjs b/scripts/dev/healthcheck.mjs index c65b0957b1..7ad4717fcf 100644 --- a/scripts/dev/healthcheck.mjs +++ b/scripts/dev/healthcheck.mjs @@ -2,9 +2,18 @@ /** * Docker healthcheck script for OmniRoute. - * Probes the /api/monitoring/health endpoint on the dashboard port. + * Probes the lightweight /healthz lifecycle endpoint on the dashboard port. * Used by Dockerfile and docker-compose files. * + * #10311 — the container HEALTHCHECK previously probed the heavy + * /api/monitoring/health path (synchronous SQLite reads + deep monitoring + * aggregation) on the same single-process event loop as catalog rebuild / + * long-context compression. Under load that probe could stall past the 5s + * timeout and flip the container `unhealthy`, restarting it mid-session and + * killing active SSE streams. /healthz is a pure in-memory lifecycle check + * with no DB access. Operators who want the deep monitoring probe can opt + * back in with OMNIROUTE_HEALTHCHECK_PATH. + * * #3151 — in some Docker network setups the server binds to a container IP and * a probe against `127.0.0.1` is not reachable, while `localhost`/`::1` (or vice * versa) is. The previous version probed ONLY `127.0.0.1` and swallowed every @@ -21,7 +30,7 @@ import { networkInterfaces } from "node:os"; const DEFAULT_HOSTS = ["127.0.0.1", "localhost", "::1"]; const DEFAULT_TIMEOUT_MS = 4000; -const DEFAULT_HEALTH_PATH = "/api/monitoring/health"; +const DEFAULT_HEALTH_PATH = "/healthz"; function normalizeBasePath(value) { const trimmed = typeof value === "string" ? value.trim() : ""; @@ -32,10 +41,34 @@ function normalizeBasePath(value) { return `/${segments.join("/")}`; } -/** Prefixes the health route with the configured Next.js basePath. */ -export function resolveHealthPath(basePathValue) { +/** + * Normalize an explicit health-check path override (OMNIROUTE_HEALTHCHECK_PATH). + * Returns "" when absent/invalid so callers fall back to DEFAULT_HEALTH_PATH. + * Mirrors normalizeBasePath's safety rules (no query/hash/backslash, no "." / + * ".." segments, must start with "/"). + */ +function normalizeHealthPath(value) { + const trimmed = typeof value === "string" ? value.trim() : ""; + if (!trimmed) return ""; + if (!trimmed.startsWith("/") || /[?#\\]/.test(trimmed)) return ""; + const segments = trimmed.split("/").filter(Boolean); + if (segments.some((segment) => segment === "." || segment === "..")) return ""; + return `/${segments.join("/")}`; +} + +/** + * Resolve the health route to probe. By default the lightweight /healthz + * lifecycle endpoint (pure in-memory, no DB reads). An explicit + * OMNIROUTE_HEALTHCHECK_PATH override opts back into the deep monitoring + * probe. The configured Next.js basePath is always prefixed. + * + * @param {string} [basePathValue] value of OMNIROUTE_BASE_PATH + * @param {string} [healthPathValue] value of OMNIROUTE_HEALTHCHECK_PATH + */ +export function resolveHealthPath(basePathValue, healthPathValue) { const basePath = normalizeBasePath(basePathValue); - return basePath ? `${basePath}${DEFAULT_HEALTH_PATH}` : DEFAULT_HEALTH_PATH; + const healthPath = normalizeHealthPath(healthPathValue) || DEFAULT_HEALTH_PATH; + return basePath ? `${basePath}${healthPath}` : healthPath; } /** @@ -115,7 +148,10 @@ async function main() { } try { - const healthPath = resolveHealthPath(process.env.OMNIROUTE_BASE_PATH); + const healthPath = resolveHealthPath( + process.env.OMNIROUTE_BASE_PATH, + process.env.OMNIROUTE_HEALTHCHECK_PATH + ); await probeHealth({ port, hosts, healthPath }); process.exit(0); } catch (err) { diff --git a/tests/unit/docker-healthcheck-3151.test.ts b/tests/unit/docker-healthcheck-3151.test.ts index a8717d521d..945c516302 100644 --- a/tests/unit/docker-healthcheck-3151.test.ts +++ b/tests/unit/docker-healthcheck-3151.test.ts @@ -19,7 +19,7 @@ const { probeHealth } = (await import("../../scripts/dev/healthcheck.mjs")) as { function startServer(host: string): Promise<{ server: http.Server; port: number }> { return new Promise((resolve, reject) => { const server = http.createServer((req, res) => { - if (req.url === "/api/monitoring/health") { + if (req.url === "/healthz") { res.writeHead(200, { "content-type": "application/json" }); res.end(JSON.stringify({ status: "ok" })); } else { diff --git a/tests/unit/docker-healthcheck-base-path.test.ts b/tests/unit/docker-healthcheck-base-path.test.ts index a49b548a55..3ad73b7485 100644 --- a/tests/unit/docker-healthcheck-base-path.test.ts +++ b/tests/unit/docker-healthcheck-base-path.test.ts @@ -2,12 +2,29 @@ import test from "node:test"; import assert from "node:assert/strict"; import { resolveHealthPath } from "../../scripts/dev/healthcheck.mjs"; -test("resolveHealthPath keeps the default route at the domain root", () => { - assert.equal(resolveHealthPath(""), "/api/monitoring/health"); - assert.equal(resolveHealthPath(undefined), "/api/monitoring/health"); +test("resolveHealthPath keeps the default lightweight /healthz route at the domain root (#10311)", () => { + assert.equal(resolveHealthPath(""), "/healthz"); + assert.equal(resolveHealthPath(undefined), "/healthz"); }); test("resolveHealthPath prefixes the health route with OMNIROUTE_BASE_PATH", () => { - assert.equal(resolveHealthPath("/omniroute/"), "/omniroute/api/monitoring/health"); - assert.equal(resolveHealthPath("/omniroute"), "/omniroute/api/monitoring/health"); + assert.equal(resolveHealthPath("/omniroute/"), "/omniroute/healthz"); + assert.equal(resolveHealthPath("/omniroute"), "/omniroute/healthz"); +}); + +test("resolveHealthPath honors an explicit OMNIROUTE_HEALTHCHECK_PATH override", () => { + assert.equal( + resolveHealthPath(undefined, "/api/monitoring/health"), + "/api/monitoring/health" + ); + assert.equal( + resolveHealthPath("/omniroute", "/api/monitoring/health"), + "/omniroute/api/monitoring/health" + ); +}); + +test("resolveHealthPath ignores an invalid/empty OMNIROUTE_HEALTHCHECK_PATH and falls back to /healthz", () => { + assert.equal(resolveHealthPath("", " "), "/healthz"); + assert.equal(resolveHealthPath("", "/../etc/passwd"), "/healthz"); + assert.equal(resolveHealthPath("", "/health?utm=1"), "/healthz"); }); diff --git a/tests/unit/probe-10311-healthcheck-lifecycle-default.test.ts b/tests/unit/probe-10311-healthcheck-lifecycle-default.test.ts new file mode 100644 index 0000000000..e106e51e88 --- /dev/null +++ b/tests/unit/probe-10311-healthcheck-lifecycle-default.test.ts @@ -0,0 +1,12 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { resolveHealthPath } from "../../scripts/dev/healthcheck.mjs"; + +// Issue #10311: the official Docker image's HEALTHCHECK (scripts/dev/healthcheck.mjs) +// must default to the lightweight lifecycle probe (/healthz) rather than the heavy +// /api/monitoring/health path (SQLite reads + deep monitoring aggregation on the same +// event loop as catalog rebuild / compression). This test asserts the expected fix. +test("resolveHealthPath defaults to the lightweight /healthz lifecycle probe, not the heavy monitoring path", () => { + assert.equal(resolveHealthPath(""), "/healthz"); + assert.equal(resolveHealthPath(undefined), "/healthz"); +}); \ No newline at end of file