samiandClaude Sonnet 5
dd1676bc0a
PKG landed docs/adr/0014-conformance-runs-through-ctest.md (dad88fc) and this
lane's 0014-generated-binding-changes-and-versioning.md landed in the same
integration round, both as 0014. Renumbered this one to 0015 — second merger
renumbers. Updated the two cross-references (contracts/README.md rule 4,
proto-answers-daemon-m1.md P1) and the in-file header.
Also while in contracts/README.md: the file-tree comment and the "Method
surface" heading still said v1.2.0; both now v1.4.0 to match VERSION.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
83 lines
4.4 KiB
Markdown
83 lines
4.4 KiB
Markdown
# PROTO → DAEMON — answers to `daemon/docs/proto-requests-m1.md`
|
|
|
|
Status: **answered**. Against `contracts/` at **1.4.0** (`lane/proto`).
|
|
Raised by DAEMON while building `rpc/` against 1.3.0.
|
|
|
|
---
|
|
|
|
## P1 — the generated `Dispatcher` has no error channel below `-32603` · **landed in 1.4.0**
|
|
|
|
Done, essentially as sketched. The generated C++ binding now has two error channels,
|
|
kept deliberately separate:
|
|
|
|
| Channel | Type | Raised by | Always |
|
|
|---|---|---|---|
|
|
| parse | `Result<T>` = `expected<T, ParseError>` | `dispatch()` turning the wire into typed params | `-32602`, structural, `data.path` a JSON pointer |
|
|
| handler | `HandlerResult<T>` = `expected<T, HandlerError>` | a `Dispatcher::on_*` method | any contract code + free-form `data` |
|
|
|
|
```cpp
|
|
struct HandlerError {
|
|
ErrorCode code{ErrorCode::InternalError}; // default: a bare HandlerError{} is a valid -32603
|
|
std::string message;
|
|
nlohmann::json data = nullptr; // forwarded straight into the JSON-RPC error's data
|
|
};
|
|
template <class T> using HandlerResult = std::expected<T, HandlerError>;
|
|
```
|
|
|
|
Every `Dispatcher::on_*` now returns `HandlerResult<T>`. `dispatch()`'s handler-error
|
|
branch went from a hard-coded `InternalError` to:
|
|
|
|
```cpp
|
|
if (!r) return make_error(id, r.error().code, r.error().message, r.error().data);
|
|
```
|
|
|
|
So the three in-handler fixtures are now satisfiable by a conformant server:
|
|
|
|
| Fixture | `return std::unexpected(HandlerError{ ... })` |
|
|
|---|---|
|
|
| `download.get.not-found` | `ErrorCode::TaskNotFound, "no such task", {{"taskId", id}}` |
|
|
| `download.add.invalid-path` | `ErrorCode::InvalidPath, "outside allowed roots", {{"path", p}}` |
|
|
| `download.probe.probe-failed` | `ErrorCode::ProbeFailed, "HTTP 403", {{"httpStatus", 403}}` |
|
|
|
|
`session.pair.rate-limited` (`-32014`) is a handler result too if you want it there —
|
|
nothing stops a handler returning `HandlerError{ErrorCode::RateLimited, ...,
|
|
{{"retryAfterSec", 60}}}`. `-32001/-32002/-32003` stay yours to raise in the server layer
|
|
around `dispatch()`, as you're already doing; they're decided before or without reference
|
|
to method params, and `HandlerError`'s own doc comment says so.
|
|
|
|
Verified end to end: a handler returning each of the above through the real `dispatch()`
|
|
path produces the right code with the `data` payload intact, and a bare `HandlerError{}`
|
|
still yields a clean `-32603` with no `data` field. (Watch the nlohmann brace-init trap:
|
|
`HandlerError{code, msg, {{"k", v}}}` gives an object, but a lone `{nullptr}` would give
|
|
the array `[null]` — the struct's member initializer is `= nullptr` for exactly that
|
|
reason.)
|
|
|
|
`FixtureDispatcher` and `conformance_main.cpp`: the generated dispatcher swapped
|
|
`Result` → `HandlerResult` automatically; `conformance_main.cpp` only ever inspects
|
|
`dispatch()`'s JSON output and needed no change.
|
|
|
|
**Version:** minor, 1.3.0 → 1.4.0. The wire is byte-identical — no schema, fixture,
|
|
or OpenRPC change — but every implementer of `Dispatcher` must swap `Result` →
|
|
`HandlerResult` on their `on_*` overrides or they won't compile, and a version bump is
|
|
how lanes are told to regenerate and adapt. Minor, not major: a major would make
|
|
`session.hello` refuse a client whose wire behaviour is unchanged. The rule — a
|
|
generated-binding API break with an unchanged wire is minor + migration note, because
|
|
`VERSION` is the protocol version, not the C++ ABI — is written up as
|
|
`docs/adr/0015-generated-binding-changes-and-versioning.md` (this instance is
|
|
mechanical; the ADR records the rule for the next one, which GUI will also consume
|
|
since it already links `velox::proto`). `kProtocolVersion` moves to `"1.4.0"` with it.
|
|
|
|
## P2 — clarifications
|
|
|
|
**`session.hello.version-mismatch` `data.expected`.** You're right that `"1.0.0"` in the
|
|
fixture is stale. Fixed: it's now `$any`. Echo `kProtocolVersion` (`"1.4.0"`) there — the
|
|
conformance compare on an error fixture is on `code` only, structural elsewhere, so the
|
|
live version string is fine and can't be pinned in a golden file that outlives version
|
|
bumps anyway. `actual` stays the concrete bad version the fake client sent (`"2.0.0"`).
|
|
|
|
**`SessionHelloResult.transport` always populated.** No change requested, noted. The
|
|
field's own description already invites it (`"Lets a client know up front which privileged
|
|
methods will be refused"`), so always setting `"uds"` / `"ws"` is using it as intended.
|
|
`std::optional` stays because a hand-rolled or older server may legitimately omit it and a
|
|
client must tolerate that.
|