Make Render health checks immediate and deterministic - #19
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Occumed79 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughThe server now supports host configuration, GET/HEAD health responses, centralized routing, static cache policies, timeouts, and guarded errors. A new health-check script probes the running server, and the build executes it after existing checks. ChangesServer health verification
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant npmBuild
participant checkServerHealth
participant server.mjs
participant HealthEndpoint
npmBuild->>checkServerHealth: run check:server
checkServerHealth->>server.mjs: spawn with HOST and port
server.mjs-->>checkServerHealth: Health endpoint ready
checkServerHealth->>HealthEndpoint: GET /health?render-probe=1
HealthEndpoint-->>checkServerHealth: 200 and ok
checkServerHealth->>HealthEndpoint: HEAD /health?render-probe=1
HealthEndpoint-->>checkServerHealth: 200 and empty body
checkServerHealth->>server.mjs: send SIGTERM
🚥 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b5c559e11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "check": "npm run check:export && npm run check:runtime", | ||
| "dev": "npm run prepare:assets && vite", | ||
| "build": "npm run prepare:assets && npm run check && vite build", | ||
| "build": "npm run prepare:assets && npm run check && vite build && npm run check:server", |
There was a problem hiding this comment.
The checked GitHub Actions pipeline still runs npx vite build directly in .github/workflows/validate.yml:56-57, so it never executes this newly appended npm run check:server. Consequently, pull requests can pass CI even when the health endpoint test fails, contrary to the intended CI readiness validation; update that workflow to call this build script or run check:server explicitly.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server.mjs (2)
173-176: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider exiting/closing on fatal server errors.
Setting
process.exitCode = 1only affects the exit code if/when the process naturally terminates. Iferrorfires after the server is already listening (e.g.,EMFILEduringaccept()), the server keeps running and accepting connections in a degraded state rather than actually stopping — which could let Render's health probe keep succeeding against a partially broken process.♻️ Proposed fix: explicitly stop the server on fatal errors
server.on('error', (error) => { console.error('Occu-Med Map server error:', error); process.exitCode = 1; + server.close(); });🤖 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 `@server.mjs` around lines 173 - 176, Update the server error handler registered by server.on('error') to explicitly stop or close the server when a fatal error occurs, while preserving the error log and nonzero exit status. Ensure the shutdown path prevents the server from continuing to accept connections after errors such as EMFILE.
26-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
PUBLIC_ORIGINinstead of trusting forwarded headers.
PUBLIC_ORIGINavoids sourcing the runtime origin from client-suppliedx-forwarded-*headers; rely only onHostor platform-trusted headers when that env var is unset.🤖 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 `@server.mjs` around lines 26 - 31, Update the origin-building function around forwardedProtocol and forwardedHost to prefer the PUBLIC_ORIGIN environment variable and return it when set. When PUBLIC_ORIGIN is unset, stop using client-supplied x-forwarded-proto and x-forwarded-host headers; derive the origin from the request Host header (or the existing localhost fallback), using only platform-trusted headers if explicitly available.
🤖 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 `@server.mjs`:
- Around line 169-171: Update the timeout assignments near server initialization
so headersTimeout is less than or equal to requestTimeout, preserving the
intended timeout ordering and ensuring the configured values satisfy Node’s
invariant.
---
Nitpick comments:
In `@server.mjs`:
- Around line 173-176: Update the server error handler registered by
server.on('error') to explicitly stop or close the server when a fatal error
occurs, while preserving the error log and nonzero exit status. Ensure the
shutdown path prevents the server from continuing to accept connections after
errors such as EMFILE.
- Around line 26-31: Update the origin-building function around
forwardedProtocol and forwardedHost to prefer the PUBLIC_ORIGIN environment
variable and return it when set. When PUBLIC_ORIGIN is unset, stop using
client-supplied x-forwarded-proto and x-forwarded-host headers; derive the
origin from the request Host header (or the existing localhost fallback), using
only platform-trusted headers if explicitly available.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 03d2eacc-9cb6-458c-80ce-666c49314592
📒 Files selected for processing (3)
package.jsonscripts/check-server-health.mjsserver.mjs
| server.requestTimeout = 30_000; | ||
| server.headersTimeout = 35_000; | ||
| server.keepAliveTimeout = 5_000; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js http server headersTimeout vs requestTimeout: does assigning both as instance properties after createServer bypass the ERR_OUT_OF_RANGE validation enforced in the constructor?
💡 Result:
No, assigning headersTimeout and requestTimeout as instance properties after the http.Server instance has been created does not trigger the ERR_OUT_OF_RANGE validation enforced in the constructor [1]. The ERR_OUT_OF_RANGE validation for these timeouts is part of the internal server option storage logic, which executes specifically during the construction of the http.Server instance (via http.createServer or new http.Server) [1][2][3]. Once the instance is initialized, these properties are exposed on the object, and you can manually overwrite them [4]. Node.js does not re-validate these instance properties when they are modified directly at runtime [1]. While it is technically possible to bypass the constructor's validation by assigning values after initialization, doing so is discouraged as it may lead to inconsistent server behavior or unexpected connection timeouts [4][5]. Node.js relies on these values to manage connection lifetimes and mitigate potential Denial-of-Service attacks, and the internal logic expects that headersTimeout is less than or equal to requestTimeout [4][6][1]. If you set them to invalid values, the server's internal connection-tracking mechanisms may not operate as intended [6][5].
Citations:
- 1: nodejs/node@3c0c5e0
- 2: http_server handles headersTimeout and requestTimeout checking wrong nodejs/node#43355
- 3: http: fix error message when specifying headerTimeout for createServer nodejs/node#44163
- 4: https://nodejs.org/api/http.html
- 5: http: fix for handling on boot timers headers and request nodejs/node#48291
- 6: Fixed http_server timeout logic for requestTimeout and headerTimeout nodejs/node#43354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Node runtime availability:\n'
node --version
printf '\nInspect relevant server.mjs lines:\n'
if [ -f server.mjs ]; then
nl -ba server.mjs | sed -n '150,180p'
else
fd -i '^server\.mjs$' -x sh -c 'echo "--- $1"; nl -ba "$1" | sed -n "150,180p"' sh {}
fi
printf '\nCheck Node http.Server behavior for constructor validation and post-init property assignment:\n'
node - <<'JS'
const http = require('http');
console.log('--- constructor with requestTimeout=30000, headersTimeout=35000 ---');
try {
const bad = http.createServer({ requestTimeout: 30000, headersTimeout: 35000 });
console.log('created:', bad.headersTimeout, bad.requestTimeout);
} catch (err) {
console.log(err.name, err.code, err.message);
}
console.log('\n--- post-init assignment with same values ---');
const good = http.createServer({ requestTimeout: 30000, headersTimeout: 35000 });
good.headersTimeout = 35000;
good.requestTimeout = 30000;
console.log('created with same values:', good.headersTimeout, good.requestTimeout);
good.headersTimeout = 35000;
console.log('after post-init headersTimeout=', good.headersTimeout);
good.requestTimeout = 30000;
console.log('after post-init requestTimeout=', good.requestTimeout);Repository: Occumed79/Map
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -u
printf 'Inspect relevant server.mjs lines:\n'
if [ -f server.mjs ]; then
sed -n '150,180p' server.mjs | cat -n
else
for f in $(fd -i '^server\.mjs$' . 2>/dev/null); do
echo "--- $f"
sed -n '150,180p' "$f" | cat -n
done
fi
printf '\nCheck Node http.Server behavior for constructor validation and post-init property assignment:\n'
node - <<'JS'
const http = require('http');
console.log('--- constructor with requestTimeout=30000, headersTimeout=35000 ---');
try {
const bad = http.createServer({ requestTimeout: 30000, headersTimeout: 35000 });
console.log('created:', bad.headersTimeout, bad.requestTimeout);
} catch (err) {
console.log(err.name, err.code, err.message);
}
console.log('\n--- post-init assignment with same values ---');
const good = http.createServer({ requestTimeout: 30000, headersTimeout: 10000 });
good.headersTimeout = 35000;
good.requestTimeout = 30000;
console.log('initial headersTimeout=', good.headersTimeout, 'requestTimeout=', good.requestTimeout);
console.log('after post-init headersTimeout=', good.headersTimeout, 'requestTimeout=', good.requestTimeout);
JSRepository: Occumed79/Map
Length of output: 1573
Keep headersTimeout below requestTimeout.
Assigning these after createServer bypasses Node’s constructor validation, but headersTimeout = 35000 still violates Node’s headersTimeout <= requestTimeout invariant with requestTimeout = 30000. The 35s header timeout can never fire before the 30s request timeout.
🤖 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 `@server.mjs` around lines 169 - 171, Update the timeout assignments near
server initialization so headersTimeout is less than or equal to requestTimeout,
preserving the intended timeout ordering and ensuring the configured values
satisfy Node’s invariant.
1 participant
Render built and started the service, but its internal
/healthprobe timed out before deployment promotion.This change:
/healthand/healthzbefore URL parsing, filesystem access, or map routing;200 okresponse for both GET and HEAD;0.0.0.0as the safe default;response.req;Summary by CodeRabbit
New Features
/healthand/healthz.GETandHEADrequests with standard responses.Bug Fixes
Tests