samiandClaude Sonnet 5
5e3e21543a
DAEMON's daemon/docs/proto-requests-m1.md P1: velox::proto::Dispatcher's
on_* methods returned Result<T> = expected<T, ParseError>, and dispatch()
mapped every handler error to -32603 InternalError. A handler had no way to
return -32010 (download.get not-found), -32011 (download.add invalid-path)
or -32013 (probe-failed) with their data payloads -- three error fixtures a
conformant server must satisfy were unreachable, blocking DAEMON's
"conformance as a server" M1 DoD.
Two error channels now, kept separate on purpose:
- parse: Result<T> / ParseError -- dispatch() failing to turn the wire into
typed params. Always -32602, always structural.
- handler: HandlerResult<T> / HandlerError -- a handler deciding the request
can't be fulfilled. Carries any ErrorCode + message + free-form data.
struct HandlerError {
ErrorCode code{ErrorCode::InternalError}; // bare {} is a valid -32603
std::string message;
nlohmann::json data = nullptr; // straight into the error's data
};
template <class T> using HandlerResult = std::expected<T, HandlerError>;
dispatch()'s handler branch is now
make_error(id, r.error().code, r.error().message, r.error().data)
instead of a hard-coded InternalError. -32001/-32002/-32003 stay the server
layer's to raise around dispatch(), as DAEMON already does.
Verified end to end against the real dispatch() path: a handler returning
TaskNotFound/InvalidPath/ProbeFailed produces -32010/-32011/-32013 with the
data object intact, and a bare HandlerError{} still yields a clean -32603
with no data field. The `= nullptr` on the member (not `{nullptr}`) matters:
brace-init of nlohmann::json from nullptr is the array [null], not JSON null.
FixtureDispatcher regenerated to HandlerResult; conformance_main.cpp only
inspects dispatch()'s JSON and needed no change. TS side is untouched beyond
the version string -- no server Dispatcher is generated there.
P2 also handled: session.hello.version-mismatch's data.expected was a stale
"1.0.0"; now $any, with a note that the error-fixture compare is on `code`
only so a server echoing kProtocolVersion there is fine.
Version: minor, 1.3.0 -> 1.4.0. Wire is byte-identical (no schema, fixture,
or OpenRPC change) but every Dispatcher implementer must swap Result ->
HandlerResult on regen, and the bump is how lanes are told to. Not an ADR:
one lane consumes this binding, it's the one that asked, and the shape is
the one they proposed. Answered in contracts/proto-answers-daemon-m1.md.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
79 lines
4.1 KiB
Markdown
79 lines
4.1 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. Not major and not an ADR: one lane consumes this binding, it's
|
|
the lane that asked, and there's no contested design here — the shape is the one you
|
|
proposed. `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.
|