fix(envoy-client): complete command and request lifecycles#4801
fix(envoy-client): complete command and request lifecycles#4801NathanFlurry wants to merge 1 commit intobreak-up/serialize-gateway-actor-keysfrom
Conversation
|
Overview This PR fixes a silent failure in handle_req_start: when the actors fetch callback returns Err, the request lifecycle was left incomplete -- no response was ever sent back to the gateway, leaving the client hanging. The fix adds send_fetch_error_response to send a proper 500 error response in the error branch. This is a correct and necessary fix. A few observations below. Issues 1. Missing content-type: application/json header The response body is JSON, but no content-type header is set. Clients that inspect the content-type before parsing the body (e.g., SDKs, error middleware) will not know to decode it as JSON. Compare to test-envoy/src/behaviors.rs:75, which sets content-type: application/json for JSON responses, and tunnel.rs::send_error_response (~line 274), which intentionally sends a plain-text body without one. Suggested addition in send_fetch_error_response: 2. JSON body format does not match canonical RivetError structure CLAUDE.md mandates: Errors use universal RivetError (group/code/message/metadata) at all boundaries. The emitted body uses a flat code field that bundles group and code together (envoy_fetch_failed), rather than the canonical separate group + code fields. The x-rivet-error header correctly uses envoy.fetch_failed (dot-notation group.code), so the JSON body should match that same structure. Without group, clients using the canonical RivetError decoder will either fail to parse or silently fall through to unstructured error handling. Suggested body format: group="envoy", code="fetch_failed", message="actor fetch failed" 3. HTTP status 500 vs 502 500 is defensible (actor runtime error), but 502 Bad Gateway is more semantically accurate since the envoy is acting as a reverse proxy and the upstream (the actor fetch handler) failed. Worth a deliberate choice either way. For reference, tunnel.rs uses 503 for the analogous "actor not found" path. Minor Notes
Summary The core fix is correct and necessary -- completing the request lifecycle on error prevents client hangs. Two items to address before merging:
|
1303d27 to
f025e6a
Compare
cf29c73 to
2cdf843
Compare
f025e6a to
c546b5e
Compare
2cdf843 to
415545d
Compare
c546b5e to
a1f99bc
Compare
415545d to
8801509
Compare
a1f99bc to
4d01526
Compare
8801509 to
31dfe70
Compare
31dfe70 to
45f80a0
Compare
4d01526 to
2f550f5
Compare
45f80a0 to
3dbc40f
Compare
2f550f5 to
2eeb6ed
Compare
2eeb6ed to
5ae4b51
Compare
3dbc40f to
1cd5bb4
Compare
1cd5bb4 to
d5c12d7
Compare
22ba82f to
0a6afdc
Compare
d5c12d7 to
436dba7
Compare
|
Landed in main via stack-merge fast-forward push. Commits are in main; closing to match. |
Preview packages published to npmInstall with: npm install rivetkit@pr-4801All packages published as Engine binary is shipped via Docker images: docker pull rivetdev/engine:slim-1822679
docker pull rivetdev/engine:full-1822679Individual packagesnpm install rivetkit@pr-4801
npm install @rivetkit/react@pr-4801
npm install @rivetkit/rivetkit-napi@pr-4801
npm install @rivetkit/workflow-engine@pr-4801 |
1 similar comment
Preview packages published to npmInstall with: npm install rivetkit@pr-4801All packages published as Engine binary is shipped via Docker images: docker pull rivetdev/engine:slim-1822679
docker pull rivetdev/engine:full-1822679Individual packagesnpm install rivetkit@pr-4801
npm install @rivetkit/react@pr-4801
npm install @rivetkit/rivetkit-napi@pr-4801
npm install @rivetkit/workflow-engine@pr-4801 |

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: