feat(rivetkit): move HTTP listener to Rust + RIVETKIT_RUNTIME_MODE env var#5132
feat(rivetkit): move HTTP listener to Rust + RIVETKIT_RUNTIME_MODE env var#5132abcxff wants to merge 2 commits into
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
🚅 Deployed to the rivet-pr-5132 environment in rivet-frontend
|
8945f57 to
df0149e
Compare
df0149e to
f4e0a85
Compare
PR Review: feat(rivetkit): move HTTP listener to Rust, auto-listen on prod start()This is a solid architectural improvement. Moving the HTTP listener to native Rust eliminates the multi-runtime (Node/Bun/Deno) abstraction in serve.ts and unifies serving under axum. The public_dir static-file fallback and PORT env-var integration are well-motivated. A few issues to address before landing: Medium: 413 error body reports size=0 when Content-Length is absent In serverless_http.rs forward_request, the .unwrap_or(0) fallback means that when the client omits Content-Length (common with chunked-encoded uploads), the response reports size=0, limit=N which is misleading since the body was clearly larger than N. Since to_bytes fails at exactly body_limit bytes, .unwrap_or(body_limit) is the more accurate fallback. Low: Silent drops in into_axum_response should log Both the invalid-header drop and the invalid-status-code fallback to 500 are silent. Both should emit tracing::warn! to make serverless-handler bugs observable at the axum layer. Low: start() auto-promote has a silent-failure path on non-Node runtimes In registry/index.ts, if listen() throws (e.g. the WASM runtime explicit error) and process is not available, the error is logged but execution continues silently. The user intended start() to boot an envoy, not fail silently. Consider propagating the error or documenting that production start() is unsupported outside Node. Low: Test polling loop should use vi.waitFor waitForResponse in listener.test.ts is a hand-rolled while (Date.now() < deadline) poll. The project convention, enforced by pnpm run check:wait-for-comments, is to use vi.waitFor with an explanatory comment on the immediately preceding line. A custom loop bypasses that check. Observation: no test coverage for RIVETKIT_AUTO_LISTEN or publicDir The two headline features have no direct test coverage. A unit test for shouldAutoListen() based on the env var and NODE_ENV would be small and prevent regressions. Nits
|
f4e0a85 to
d07e88c
Compare
d07e88c to
106d943
Compare
106d943 to
992bb34
Compare
992bb34 to
cc44f96
Compare
Review: feat(rivetkit): move HTTP listener to Rust, auto-listen on prod start()Good overall direction — consolidating the HTTP listener into Rust eliminates the Hono/Node-server cross-platform patchwork and the fragile CRITICAL: Duplicate
The new method is strictly weaker — HIGH:
HIGH: Documented The JSDoc on MEDIUM: Double
MEDIUM: Removed request body size from error message
LOW: In LOW: LOW: Tests Test coverage of the new units is solid ( Overall the Rust Axum listener is cleanly structured — the |
cc44f96 to
8331535
Compare
8331535 to
8086664
Compare
8086664 to
21b19a1
Compare
21b19a1 to
6df2c7f
Compare
Code Review: feat(rivetkit): move HTTP listener to Rust + RIVETKIT_RUNTIME_MODE env varOverviewThis PR replaces the Hono/ Bugs / CorrectnessWelcome message prints before the listener is bound In
If the container has no Design / ArchitectureRemoving The error changed from "Received {size} bytes, limit is {limit} bytes" to "Exceeded limit of {limit} bytes." The received size is often the most actionable field when debugging an oversized request. This is a regression in observability.
The function takes an opaque Silent header drops in Invalid response headers are silently discarded in the Double-promise caching comment in The comment "Cache on both promise fields so the shutdown drain sees Mode A and B" explains the intent but not the contract: which shutdown path reads each field, and what breaks if they diverge. A link to the shutdown drain logic or one more sentence of context would prevent a future reader from treating the dual-assignment as a copy-paste error. Testing
CLAUDE.md requires a one-line No Rust unit tests for The new module has no coverage under Duplicate registry setup across E2E tests The two Minor / Nits
SummaryThe architectural move to Rust+Axum is well-structured, the new env-var API is clean, and the client-abort signaling via |
89abcb3 to
2e5c4ef
Compare
2e5c4ef to
66ba4c8
Compare
66ba4c8 to
d390612
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: