chore(rpc): adopt the prpc empty-body encoding for unit responses - #987
chore(rpc): adopt the prpc empty-body encoding for unit responses#987kvinwang wants to merge 2 commits into
Conversation
prpc-build 0.7.0 encodes a `google.protobuf.Empty` response as an empty JSON body instead of the literal `null`, matching what the protobuf codec has always done. Decode responses through `prpc::codec::decode_json_from_slice` so an empty body maps back to the unit type. Upstream: Phala-Network/prpc#1
aaf3b64 to
4abe14c
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates dstack’s RPC stack to align JSON unit (google.protobuf.Empty / ()) responses with upstream prpc-build behavior (empty response body instead of literal null), and adjusts the ra-rpc client to decode that format correctly.
Changes:
- Bump
prpcto0.6.2andprpc-buildto0.7.0(workspace deps). - Update
ra-rpc’sRequestClientimplementation to decode JSON responses viaprpc::codec::decode_json_from_slice, which maps an empty body back to the unit type. - Refresh
Cargo.lockto reflect the dependency updates.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| dstack/ra-rpc/src/client.rs | Switch JSON decoding to prpc::codec::decode_json_from_slice to support empty-body unit responses. |
| dstack/Cargo.toml | Bump workspace prpc / prpc-build versions to pick up the new unit-response encoding. |
| dstack/Cargo.lock | Lockfile update for the prpc / prpc-build version changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| prpc = "0.6.2" | ||
| prpc-build = "0.7.0" |
There was a problem hiding this comment.
Good catch, this was a real runtime break and my compile checks could not have caught it — serde_json::from_slice::<()> type-checks fine and only fails on the empty input at run time.
Fixed in 9887987: PrpcClient now decodes through prpc::codec::decode_json_from_slice as well. There are exactly two RequestClient impls in the tree (ra-rpc::RaClient and http-client::PrpcClient); both are updated now, and I left a comment at each call site so a future edit does not quietly revert to serde_json::from_slice.
`PrpcClient` is the second `RequestClient` impl in the tree and still used `serde_json::from_slice`, so an empty unit body would fail to parse at runtime. `dstack-cli-core`'s `stop_vm` / `remove_vm` go through it and would have reported a failure for a request that actually succeeded. Compile checks cannot catch this: `serde_json::from_slice::<()>` type-checks fine and only fails on the empty input at runtime.
|
Closing: the upstream change this depends on has been reverted.
Rationale: the empty-body encoding fixed nothing that was broken — the EOF bug in #875 was entirely fixed by the shutdown ordering, which is merged. What it bought was codec symmetry with protobuf; what it cost was a breaking wire change across 26 RPCs, a runtime break in |
Follow-up to #875, which dropped a transport-layer workaround that byte-matched
nullinthe Rocket responder. The problem is fixed properly upstream instead:
Phala-Network/prpc#1.
What changes
prpc-build0.7.0 encodes agoogle.protobuf.Emptyresponse as an empty JSON bodyinstead of the literal
null. The decision is made in codegen from the response type, soa method that legitimately returns a JSON
nullvalue is unaffected, and message fields oftype
Emptystill serialize asnullinside the object — only the top-level body changes.The protobuf codec is untouched; it has always encoded a unit as zero bytes.
Both
RequestClientimpls in the tree decode throughprpc::codec::decode_json_from_slice,which maps an empty body back to the unit type:
ra-rpc::RaClienthttp-client::PrpcClient— missed in the first revision, caught in review.dstack-cli-core'sstop_vm/remove_vmgo through it and would have reported a failure for a request thatactually succeeded. A compile check cannot catch this:
serde_json::from_slice::<()>type-checks fine and only fails on the empty input at run time.
An empty body for any other message type stays an error.
Affected wire format
Every unit-returning JSON RPC now responds with an empty body rather than
null:StartVm,StopVm,RemoveVm,ShutdownVm,ResizeVm,SvStop,SvRemove,PullRegistryImage, ...Exit,ReloadCert, and the admin RPCsOnboard.FinishClient audit:
ra-rpc::RaClienthttp-client::PrpcClientvmmRpc(StartVm/StopVm/RemoveVm/ShutdownVm)application/octet-stream, protobuf pathbaseRpcCall(SvStop/SvRemove)vmm-cli.pysend-rpc-request.tsJSON.parse(data)would throw, but guest-agent exposes no unit-returning method. Should be hardened before one is addedVerification
cargo check/cargo clippyfor kms, vmm, gateway, guest-agent, ra-rpc, http-client anddstack-cli-core: clean.
cargo test -p ra-rpc: pass.kms.rs/vmm.rs:dispatch_json_requestemitsOk(Vec::new())for unit methods,dispatch_request(protobuf) is byte-for-byteunchanged.