sami/vdm
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

proto: give the generated C++ Dispatcher a real error channel (P1, 1.4.0)

Browse Source 
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
This commit is contained in:
sami
2026-09-10 15:10:13 +04:00
co-authored by Claude Sonnet 5
parent 9dce588456
commit 5e3e21543a
15 changed files with 298 additions and 190 deletions
Download Patch File Download Diff File Expand all files Collapse all files
contracts/README.md
+10 -8
View File
@@ -3,16 +3,18 @@
**This directory is the interface between every lane.** Owner: agent **PROTO**.
Nobody else commits here. Everybody else *generates from* here.
> ## Status: **v1.3.0** (frozen at v1.0.0 on 2026-09-09; minor bumps since)
> ## Status: **v1.4.0** (frozen at v1.0.0 on 2026-09-09; minor bumps since)
>
> **v1.0.0** froze 38 methods, 9 events, 26 named types. **v1.1.0** widened `bufferBytes`
> bounds and added the segment-budget settings (`docs/adr/0012-...`). **v1.2.0** added
> `download.provideAuth`, F2's credential return path. **v1.3.0** (current) widens when
> `event.task.state.error` / `TaskSummary.error` are populated to also cover a `paused`
> the daemon entered unilaterally (`auth_required`, `server_file_changed`, disk full),
> not just `failed`/`retry_wait` — the wire shape is unchanged (`error` was already
> `TaskError | null`), only the description of when it's set. Landed for DAEMON's
> `docs/adr/0013-task-state-machine-ownership.md`. See also `docs/adr/0005-...` for the
> bounds and added the segment-budget settings. **v1.2.0** added `download.provideAuth`
> (F2). **v1.3.0** widened when `error` is populated on a state change to cover a
> daemon-initiated `paused` (for `docs/adr/0013-...`). **v1.4.0** (current) is a
> **C++-binding-only** change: the generated `Dispatcher` gains a `HandlerError` /
> `HandlerResult<T>` error channel so a handler can return `-32010` / `-32011` /
> `-32013` with their `data` payloads instead of collapsing to `-32603`. The wire is
> byte-identical — no schema or fixture change — but any `Dispatcher` implementer
> must swap `Result` → `HandlerResult` on regen. Answered in
> `contracts/proto-answers-daemon-m1.md`. See also `docs/adr/0005-...` for the
> versioning rule and `docs/adr/0010-...` for the failure taxonomy and segment ranges.
>
> Lane requests are answered in writing: `contracts/proto-answers-m1.md` responds to
contracts/VERSION
+1 -1
View File
@@ -1 +1 @@
1.3.0
1.4.0
contracts/codegen/gen_cpp.py
+31 -4
View File
@@ -7,6 +7,13 @@ Design notes that matter to the CORE and DAEMON lanes:
not used and not emitted. Parsing goes through `velox::proto::parse<T>(json)` which
returns `std::expected<T, ParseError>`, so a malformed frame from the wire is an ordinary
value the RPC loop handles, not a throw unwinding through the transfer path.
* **Two error channels, kept separate.** `parse<T>() -> Result<T>` (i.e. `expected<T,
ParseError>`) is the wire failing to become typed params — always `-32602`, always
structural. A `Dispatcher::on_*` handler returns `HandlerResult<T>` (i.e. `expected<T,
HandlerError>`), which carries any contract error code plus a free-form `data` object,
so a handler can answer `-32010` `{taskId}`, `-32011` `{path}`, `-32013` `{httpStatus}`
and so on. `dispatch()` forwards the handler's code/message/data straight into the
JSON-RPC error object.
* **Serialisation is ADL `to_json`,** so `nlohmann::json j = task;` works as expected.
Only the outbound direction is allowed to be implicit; the wire is never trusted.
* **`libveloxproto`, not `libveloxcore`.** This code includes nlohmann/json, which
@@ -285,16 +292,37 @@ def emit_header(c: Contract) -> str:
"nlohmann::json make_result(const nlohmann::json& id, nlohmann::json result);",
"nlohmann::json make_notification(Event e, nlohmann::json params);",
"",
"/// A handler's own failure — as opposed to ParseError, which is the wire failing",
"/// to become typed params. Carries any contract error code, a message, and a",
"/// free-form `data` object that goes straight into the JSON-RPC error's `data`",
"/// field: `{\"taskId\": ...}` for TaskNotFound, `{\"path\": ...}` for InvalidPath,",
"/// `{\"httpStatus\": ...}` for ProbeFailed. `code` defaults to InternalError so a",
"/// handler that sets only a message still produces a valid error response.",
"///",
"/// -32001/-32002/-32003 are the server layer's to raise around dispatch(), not a",
"/// handler's: they are decided before or without reference to method params.",
"struct HandlerError {",
" ErrorCode code{ErrorCode::InternalError};",
" std::string message;",
" // `= nullptr`, not `{nullptr}`: brace-init of nlohmann::json from nullptr",
" // yields the array [null], not JSON null. make_error() drops a null data.",
" nlohmann::json data = nullptr;",
"};",
"",
"template <class T>",
"using HandlerResult = std::expected<T, HandlerError>;",
"",
"/// One virtual per method. The daemon implements this; `dispatch` below does the",
"/// envelope handling, the transport check and the parameter parsing, so a handler",
"/// only ever sees a validated, typed params struct.",
"/// only ever sees a validated, typed params struct. Return `std::unexpected(",
"/// HandlerError{...})` to answer with a specific error code and data.",
"class Dispatcher {",
"public:",
" virtual ~Dispatcher() = default;",
""]
for m in c.methods:
o += doc_comment(m.doc, " ")
o.append(f" virtual Result<{cpp_type(m.result)}> {handler_name(m.name)}(const {cpp_type(m.params)}& params) = 0;")
o.append(f" virtual HandlerResult<{cpp_type(m.result)}> {handler_name(m.name)}(const {cpp_type(m.params)}& params) = 0;")
o.append("")
o += ["};", "",
"/// Parse one JSON-RPC request, route it, and return the response to write back.",
@@ -579,8 +607,7 @@ def emit_dispatch(c: Contract) -> list[str]:
' nlohmann::json{{"path", p.error().path}});',
f" auto r = handler.{handler_name(m.name)}(*p);",
" if (!r)",
" return make_error(id, ErrorCode::InternalError, r.error().message,",
' nlohmann::json{{"path", r.error().path}});',
" return make_error(id, r.error().code, r.error().message, r.error().data);",
" nlohmann::json out = *r;",
" return make_result(id, std::move(out));",
" }",
contracts/codegen/gen_cpp_conformance.py
+13 -4
View File
@@ -44,7 +44,7 @@ def main() -> int:
# velox::conformance, so the names need qualifying.
pt, rt = "proto::" + cpp_type(m.params), "proto::" + cpp_type(m.result)
o += [
f" proto::Result<{rt}> {handler_name(m.name)}(const {pt}& params) override {{",
f" proto::HandlerResult<{rt}> {handler_name(m.name)}(const {pt}& params) override {{",
" (void)params;",
f' return golden<{rt}>("{m.name}");',
" }",
@@ -53,12 +53,21 @@ def main() -> int:
o += [
"private:",
" // Every fixture-backed handler only ever succeeds. A missing or unparseable",
" // fixture is a bug in the suite, not a contract outcome, so it surfaces as",
" // InternalError rather than being dressed up as a real error code.",
" template <class T>",
" proto::Result<T> golden(const std::string& method) {",
" proto::HandlerResult<T> golden(const std::string& method) {",
" const nlohmann::json* value = results_(method);",
" if (value == nullptr)",
' return std::unexpected(proto::ParseError{method, "no fixture for this method"});',
" return proto::parse<T>(*value, method);",
" return std::unexpected(proto::HandlerError{",
' proto::ErrorCode::InternalError, "no fixture for method " + method});',
" auto parsed = proto::parse<T>(*value, method);",
" if (!parsed)",
" return std::unexpected(proto::HandlerError{",
" proto::ErrorCode::InternalError,",
' "fixture for " + method + " failed to parse: " + parsed.error().message});',
" return std::move(*parsed);",
" }",
"",
" std::function<const nlohmann::json*(const std::string&)> results_;",
contracts/fixtures/errors/session.hello.version-mismatch.json
+3 -2
View File
@@ -18,7 +18,7 @@
"code": -32001,
"message": "protocol major version mismatch: daemon speaks 1.x, client speaks 2.x",
"data": {
"expected": "1.0.0",
"expected": "$any",
"actual": "2.0.0"
}
}
@@ -27,7 +27,8 @@
"the connection is closed after this reply; no method is served on a mismatched major",
"a differing minor or patch is accepted, never refused",
"the message is safe to show a user verbatim",
"the version check is transport-independent; this is replayed on the Unix socket so it is not masked by -32002"
"the version check is transport-independent; this is replayed on the Unix socket so it is not masked by -32002",
"data.expected is the daemon's own current protocol version string (kProtocolVersion), not a bare major and not pinnable in a golden file -- the conformance compare on error payloads is on `code` only, structural elsewhere, so echoing the live version is fine"
],
"transport": "uds"
}
contracts/openrpc.json
+1 -1
View File
@@ -2,7 +2,7 @@
"openrpc": "1.2.6",
"info": {
"title": "Velox Download Manager",
"version": "1.3.0",
"version": "1.4.0",
"description": "The wire contract between veloxd and every client: the Qt GUI, the CLI, the native-messaging host and the Firefox extension. One JSON-RPC 2.0 payload set over four framings; only the framing differs.\n\nGENERATED from contracts/schema/ by contracts/codegen/gen_openrpc.py. Do not edit by hand.",
"license": {
"name": "See repository LICENSE"
contracts/proto-answers-daemon-m1.md
+78
View File
@@ -0,0 +1,78 @@
# 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.