From e585113daf17a9555ab0a1fb3f2704c6b0f1a59f Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 15:31:00 +0400 Subject: [PATCH] =?UTF-8?q?daemon:=20adopt=20HandlerResult=20=E2=80=94?= =?UTF-8?q?=20real=20error=20codes=20from=20handlers=20(contracts/=201.4.0?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto main at 1.4.0. The regenerated Dispatcher returns HandlerResult = expected (ADR 0014); the covariant-return break on all 39 overrides is the swap predicted in daemon/docs/proto-requests-m1.md P1. - dispatcher.hpp/.cpp: Result -> HandlerResult on every override; not_implemented() now returns HandlerError{InternalError, ...} rather than a ParseError forwarded as -32603. - download.get: returns -32010 TaskNotFound with data.taskId. Not a placeholder — with no store, every id is genuinely not-found, which is the real answer for contracts/ fixture download.get.not-found. It becomes a store lookup when store/ is wired in. - uds_roundtrip: the -32603-collapse guard is now a -32010 + data.taskId assertion, the regression guard the P1 note promised. download.add (-32011) and download.probe (-32013) stay InternalError until they have real bodies (canonicalization / probe); they get their fixture codes when that logic lands. All 24 tests green; uds_roundtrip TSan-clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/src/rpc/dispatcher.cpp | 97 +++++++++++++++-------------- daemon/src/rpc/dispatcher.hpp | 78 +++++++++++------------ daemon/tests/uds_roundtrip_test.cpp | 12 ++-- 3 files changed, 97 insertions(+), 90 deletions(-) diff --git a/daemon/src/rpc/dispatcher.cpp b/daemon/src/rpc/dispatcher.cpp index 4063387..39b76f4 100644 --- a/daemon/src/rpc/dispatcher.cpp +++ b/daemon/src/rpc/dispatcher.cpp @@ -6,12 +6,13 @@ namespace proto = velox::proto; namespace { -// The generated Result carries only proto::ParseError, whose {path, message} the -// generated dispatch() forwards as -32603 data. Until P1 (daemon/docs/proto-requests-m1.md) -// gives handlers a real error channel, an unimplemented method says so plainly here. +// A method whose body arrives with the store / scheduler. Answers -32603 with a clear +// message through the generated HandlerError channel (contracts/ 1.4.0, ADR 0014). template -proto::Result not_implemented(const char* method) { - return std::unexpected(proto::ParseError{method, "not implemented in this build"}); +proto::HandlerResult not_implemented(const char* method) { + return std::unexpected(proto::HandlerError{ + proto::ErrorCode::InternalError, + std::string("not implemented in this build: ") + method}); } } // namespace @@ -20,24 +21,24 @@ proto::Result not_implemented(const char* method) { // Kept as explicit stubs so a direct dispatch() caller (a test, a future in-process client) // gets a clear answer rather than undefined behaviour from a missing override. -proto::Result +proto::HandlerResult VeloxDispatcher::on_session_hello(const proto::SessionHelloParams&) { return not_implemented("session.hello"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_session_pair(const proto::SessionPairParams&) { return not_implemented("session.pair"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_session_subscribe(const proto::SessionSubscribeParams&) { return not_implemented("session.subscribe"); } // --- download.list : an empty table, so a client can connect and render --------------- -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_list(const proto::DownloadListParams&) { proto::DownloadListResult r; r.total = 0; @@ -46,141 +47,145 @@ VeloxDispatcher::on_download_list(const proto::DownloadListParams&) { // --- everything else : not implemented until the store and scheduler land ------------- -proto::Result +proto::HandlerResult VeloxDispatcher::on_capture_getRules(const proto::CaptureGetRulesParams&) { return not_implemented("capture.getRules"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_capture_offer(const proto::CaptureOfferParams&) { return not_implemented("capture.offer"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_category_list(const proto::CategoryListParams&) { return not_implemented("category.list"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_category_remove(const proto::CategoryRemoveParams&) { return not_implemented("category.remove"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_category_upsert(const proto::CategoryUpsertParams&) { return not_implemented("category.upsert"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_add(const proto::DownloadSpec&) { return not_implemented("download.add"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_addBatch(const proto::DownloadAddBatchParams&) { return not_implemented("download.addBatch"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_cancel(const proto::DownloadCancelParams&) { return not_implemented("download.cancel"); } -proto::Result -VeloxDispatcher::on_download_get(const proto::DownloadGetParams&) { - return not_implemented("download.get"); +proto::HandlerResult +VeloxDispatcher::on_download_get(const proto::DownloadGetParams& params) { + // No store is wired yet, so no task exists and every id is genuinely not-found. This + // is the real -32010 answer (contracts/ error fixture download.get.not-found), not a + // placeholder; it becomes a store lookup when store/ is wired in. + return std::unexpected(proto::HandlerError{proto::ErrorCode::TaskNotFound, "no such task", + nlohmann::json{{"taskId", params.taskId}}}); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_pause(const proto::DownloadPauseParams&) { return not_implemented("download.pause"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_probe(const proto::DownloadProbeParams&) { return not_implemented("download.probe"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_provideAuth(const proto::DownloadProvideAuthParams&) { return not_implemented("download.provideAuth"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_refreshUrl(const proto::DownloadRefreshUrlParams&) { return not_implemented("download.refreshUrl"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_remove(const proto::DownloadRemoveParams&) { return not_implemented("download.remove"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_resume(const proto::DownloadResumeParams&) { return not_implemented("download.resume"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_start(const proto::DownloadStartParams&) { return not_implemented("download.start"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_download_update(const proto::DownloadUpdateParams&) { return not_implemented("download.update"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_grabber_harvest(const proto::GrabberHarvestParams&) { return not_implemented("grabber.harvest"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_grabber_start(const proto::GrabberStartParams&) { return not_implemented("grabber.start"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_grabber_status(const proto::GrabberStatusParams&) { return not_implemented("grabber.status"); } -proto::Result VeloxDispatcher::on_limiter_get(const proto::LimiterGetParams&) { +proto::HandlerResult VeloxDispatcher::on_limiter_get(const proto::LimiterGetParams&) { return not_implemented("limiter.get"); } -proto::Result VeloxDispatcher::on_limiter_set(const proto::Limiter&) { +proto::HandlerResult VeloxDispatcher::on_limiter_set(const proto::Limiter&) { return not_implemented("limiter.set"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_media_addVariant(const proto::MediaAddVariantParams&) { return not_implemented("media.addVariant"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_media_listVariants(const proto::MediaListVariantsParams&) { return not_implemented("media.listVariants"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_queue_list(const proto::QueueListParams&) { return not_implemented("queue.list"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_queue_reorder(const proto::QueueReorderParams&) { return not_implemented("queue.reorder"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_queue_start(const proto::QueueStartParams&) { return not_implemented("queue.start"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_queue_stop(const proto::QueueStopParams&) { return not_implemented("queue.stop"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_queue_upsert(const proto::QueueUpsertParams&) { return not_implemented("queue.upsert"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_rules_list(const proto::RulesListParams&) { return not_implemented("rules.list"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_rules_upsert(const proto::RulesUpsertParams&) { return not_implemented("rules.upsert"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_schedule_get(const proto::ScheduleGetParams&) { return not_implemented("schedule.get"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_schedule_set(const proto::ScheduleSetParams&) { return not_implemented("schedule.set"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_settings_get(const proto::SettingsGetParams&) { return not_implemented("settings.get"); } -proto::Result +proto::HandlerResult VeloxDispatcher::on_settings_set(const proto::SettingsSetParams&) { return not_implemented("settings.set"); } diff --git a/daemon/src/rpc/dispatcher.hpp b/daemon/src/rpc/dispatcher.hpp index 5e7e97a..1e21e70 100644 --- a/daemon/src/rpc/dispatcher.hpp +++ b/daemon/src/rpc/dispatcher.hpp @@ -21,82 +21,82 @@ namespace velox::daemon::rpc { class VeloxDispatcher final : public velox::proto::Dispatcher { public: - velox::proto::Result + velox::proto::HandlerResult on_capture_getRules(const velox::proto::CaptureGetRulesParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_capture_offer(const velox::proto::CaptureOfferParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_category_list(const velox::proto::CategoryListParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_category_remove(const velox::proto::CategoryRemoveParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_category_upsert(const velox::proto::CategoryUpsertParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_add(const velox::proto::DownloadSpec&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_addBatch(const velox::proto::DownloadAddBatchParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_cancel(const velox::proto::DownloadCancelParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_get(const velox::proto::DownloadGetParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_list(const velox::proto::DownloadListParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_pause(const velox::proto::DownloadPauseParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_probe(const velox::proto::DownloadProbeParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_provideAuth(const velox::proto::DownloadProvideAuthParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_refreshUrl(const velox::proto::DownloadRefreshUrlParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_remove(const velox::proto::DownloadRemoveParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_resume(const velox::proto::DownloadResumeParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_start(const velox::proto::DownloadStartParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_download_update(const velox::proto::DownloadUpdateParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_grabber_harvest(const velox::proto::GrabberHarvestParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_grabber_start(const velox::proto::GrabberStartParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_grabber_status(const velox::proto::GrabberStatusParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_limiter_get(const velox::proto::LimiterGetParams&) override; - velox::proto::Result on_limiter_set(const velox::proto::Limiter&) override; - velox::proto::Result + velox::proto::HandlerResult on_limiter_set(const velox::proto::Limiter&) override; + velox::proto::HandlerResult on_media_addVariant(const velox::proto::MediaAddVariantParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_media_listVariants(const velox::proto::MediaListVariantsParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_queue_list(const velox::proto::QueueListParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_queue_reorder(const velox::proto::QueueReorderParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_queue_start(const velox::proto::QueueStartParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_queue_stop(const velox::proto::QueueStopParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_queue_upsert(const velox::proto::QueueUpsertParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_rules_list(const velox::proto::RulesListParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_rules_upsert(const velox::proto::RulesUpsertParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_schedule_get(const velox::proto::ScheduleGetParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_schedule_set(const velox::proto::ScheduleSetParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_session_hello(const velox::proto::SessionHelloParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_session_pair(const velox::proto::SessionPairParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_session_subscribe(const velox::proto::SessionSubscribeParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_settings_get(const velox::proto::SettingsGetParams&) override; - velox::proto::Result + velox::proto::HandlerResult on_settings_set(const velox::proto::SettingsSetParams&) override; }; diff --git a/daemon/tests/uds_roundtrip_test.cpp b/daemon/tests/uds_roundtrip_test.cpp index 39f8ea9..a221f9a 100644 --- a/daemon/tests/uds_roundtrip_test.cpp +++ b/daemon/tests/uds_roundtrip_test.cpp @@ -148,17 +148,19 @@ void run() { ::close(c); } - // --- download.get -> -32603 for now: documents the P1 codegen gap --------------- - // (proto-requests-m1.md P1: handlers cannot yet return -32010. When P1 lands this - // check flips to -32010 and is the regression guard for it.) + // --- download.get on an unknown id -> -32010, with data.taskId ------------------ + // (contracts/ error fixture download.get.not-found; reachable now that 1.4.0 gave + // handlers the HandlerError channel — ADR 0014.) { const int c = connect_client(sock); + const std::string missing = "00000000-0000-4000-8000-000000000000"; const json reply = call(c, {{"jsonrpc", "2.0"}, {"id", 6}, {"method", "download.get"}, - {"params", {{"taskId", "00000000-0000-4000-8000-000000000000"}}}}); + {"params", {{"taskId", missing}}}}); CHECK(reply.contains("error")); - CHECK_EQ(reply["error"]["code"].get(), -32603); + CHECK_EQ(reply["error"]["code"].get(), -32010); + CHECK_EQ(reply["error"]["data"]["taskId"].get(), missing); ::close(c); }