🪦 feat: Request Tombstones, Memory Watermarks & Server Error Logging - #102
🪦 feat: Request Tombstones, Memory Watermarks & Server Error Logging#102dustinhealy wants to merge 5 commits into
Conversation
… bun server The server's only log output is its two startup lines. The HTTP metrics increment on completion, so a request that kills the process mid-flight is invisible to them, an uncaught handler throw produces no output at all, and there is no signal for memory growth between metric scrapes. Diagnosing a crash or an OOMKill from this is guesswork. Adds three narrow instruments to the serve path. Request lines log at arrival (before the handler) and completion, so a fatal request leaves its method and path as the process's last words; paths are logged without query strings, truncated, kubelet probes are skipped, and a per-window cap stops a request flood from amplifying into a log flood while still surfacing the suppressed count. A memory watermark logs RSS threshold crossings on a 10s tick to timestamp allocation bursts even when no request is in flight. An error hook on Bun.serve logs unhandled handler throws. On by default; disable with ADMIN_PANEL_REQUEST_LOG=false. Thresholds configurable via ADMIN_PANEL_MEMORY_LOG_THRESHOLDS_MB.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@codex review |
![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: 1637d38c56
ℹ️ 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".
…n Drain A request could claim to be a kubelet probe and suppress both its arrival and completion lines, so a failing or hostile request evaded the new diagnostics entirely. The real probe path already bypasses this wrapper, so the header was buying nothing; suppression now also requires the request to target /health. Completion was reported when the handler returned, which for a streamed body such as the audit-log CSV export happens before any bytes are delivered. That logged a 200 for transfers that could still fail. Report once the body drains, and mark the line when the upstream stream errors instead. Also return the highest crossed memory threshold rather than the last one the caller happened to list, move the exported logging interfaces into the types barrel, and document both new environment variables.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 13662ef. Configure here.
|
@codex re-review |
reportOnBodyComplete has three exits that each report an outcome, and the cancel path races whatever read is in flight. Make the single-report invariant explicit rather than resting on the stream machinery happening to suppress the loser of that race, since a duplicated completion line is exactly what muddies triage on the path this logging exists to serve.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13662ef0fc
ℹ️ 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".
A rejecting handler left only an arrival line: the await exited before a status was recorded, so the request never got a completion tombstone and Bun's error line carries no method or path. Catch it in the wrapper, log the 500 with the request identity, record the metric, and rethrow so the error response is unchanged. A HEAD response whose handler still produced a body never completed either, as Bun discards that body without reading or cancelling it, leaving the monitored stream unsettled. Report HEAD status immediately instead of waiting on a stream the server will not consume.
|
@codex re-review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd3ffeb60
ℹ️ 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".
Monitoring a response body replaces it with a JavaScript stream, which for a Bun.file body costs the sendfile path and the Content-Length Bun derives from the file. A Bun.file response carries no eager content-length header, so the wrapped copy had none either and every JS, CSS, font, and image turned into a chunked transfer, on by default with request logging. Static routes now report completion as soon as the status is known and hand back the untouched response. That trades exact delivery timing on assets Bun serves natively for keeping that native path, and leaves body monitoring where it was added for: the proxied streams on the dynamic route.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
The bun server's only log output is its two startup lines, which leaves crashes and OOMKills undiagnosable: the HTTP metrics increment on completion, so a request that kills the process mid-flight is invisible to them, an uncaught handler throw produces no output at all, and nothing captures memory growth between metric scrapes.
Adds three narrow instruments to the serve path (
server.ts, pure helpers insrc/server/logging.ts):[req] GET /pathat arrival (before the handler runs, so a fatal request leaves its method and path as the process's last words) plus[req] GET /path 200 12mson completion. An arrival line with no matching completion identifies the request a process died holding. Paths log without query strings (they can carry OAuth exchange codes) and truncate at 200 chars. Kubelet probes (kube-probe/user-agent) are skipped so steady-state logs stay near-silent. A per-window cap (200 requests / 10s) stops a request flood from amplifying into a log flood, surfacing[req] suppressed N requests in prior windowinstead.[mem] rss=391Mi heapUsed=213Mi (crossed 384Mi)on each upward threshold crossing (defaults 256/384/448 MiB, configurable viaADMIN_PANEL_MEMORY_LOG_THRESHOLDS_MB), re-arming when memory drops back below a threshold. This timestamps allocation bursts even when no request is in flight — the deciding evidence between a pathological request and runtime GC behavior.Bun.servehad noerror()handler, so an uncaught throw in the SSR handler produced no log line at all. It now logs the message and stack and returns a plain 500.On by default; disable with
ADMIN_PANEL_REQUEST_LOG=false. Prometheus metrics,/metrics, and/healthbehavior unchanged.Change Type
Testing
22 new unit tests (probe detection, truncation, flood-guard windows, watermark hysteresis); full suite (821 tests) passing;
tsc --noEmit,eslint, and a from-scratch production build clean.Verified live against the production build (
bun run build+bun run start):GET /login?redirect=/...logs as[req] GET /login— query string never appearskube-probe/1.29requests produce zero lines...(truncated)marker[req] suppressed 30 requests in prior windowwhen the next window opensADMIN_PANEL_REQUEST_LOG=falseproduces zero request lines under real trafficTest Configuration:
SESSION_SECRETsetADMIN_PANEL_MEMORY_LOG_THRESHOLDS_MB=50,80so thresholds sit below the server's baseline RSSChecklist