From e8587b52d979aa86f28850343c8e1540993b7040 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 13:13:15 +0000 Subject: [PATCH 1/7] Build RpcError through one constructor surface RpcError::new, a constructor per code, forbidden and approval_required over two core constants, and with_message, with_details, with_cause and with_causes. Non-test struct literals and the one-line helpers that wrapped them now call these. The serialized form is unchanged. --- .../crates/ployz-core/src/domain/data_loss.rs | 8 +- core/crates/ployz-core/src/rpc.rs | 170 +++++++++++- .../crates/ployz-core/src/storage_capacity.rs | 16 +- core/crates/ployz-core/src/volume_copy.rs | 8 +- core/crates/ployz-core/src/volume_size.rs | 8 +- core/crates/ployz-core/src/volume_switch.rs | 8 +- core/crates/ployz-sdk/src/lib.rs | 22 +- core/crates/ployz-store/src/build.rs | 10 +- .../ployz-store/src/deployment/admit.rs | 18 +- core/crates/ployz-store/src/error.rs | 19 +- core/crates/ployz-testkit/src/fake_fleet.rs | 10 +- core/crates/ployz/src/approval.rs | 5 +- core/crates/ployz/src/cloud_account.rs | 55 ++-- core/crates/ployz/src/cluster.rs | 193 +++++-------- core/crates/ployz/src/connect.rs | 59 ++-- core/crates/ployz/src/deploy.rs | 52 ++-- core/crates/ployz/src/deploy/cleanup.rs | 87 ++---- core/crates/ployz/src/deploy/exec/mod.rs | 68 ++--- core/crates/ployz/src/deploy/exec_tests.rs | 2 +- core/crates/ployz/src/deploy/pipeline.rs | 30 +- core/crates/ployz/src/deploy/planning.rs | 33 +-- core/crates/ployz/src/drain/retirement.rs | 43 +-- core/crates/ployz/src/enrollment.rs | 17 +- core/crates/ployz/src/failure.rs | 16 +- core/crates/ployz/src/global_catch_up.rs | 50 +--- core/crates/ployz/src/global_slot.rs | 16 +- core/crates/ployz/src/handlers/cloud_read.rs | 2 +- .../ployz/src/handlers/deploy/progress.rs | 29 +- core/crates/ployz/src/handlers/mcp.rs | 26 +- core/crates/ployz/src/handlers/server.rs | 4 +- core/crates/ployz/src/handlers/service/mod.rs | 14 +- core/crates/ployz/src/handlers/teardown.rs | 12 +- core/crates/ployz/src/ingress.rs | 35 +-- core/crates/ployz/src/operator/history.rs | 15 +- core/crates/ployz/src/private_access.rs | 37 +-- core/crates/ployz/src/published/mod.rs | 2 +- core/crates/ployz/src/sdk.rs | 259 +++++++----------- core/crates/ployz/src/sdk/agent.rs | 24 +- core/crates/ployz/src/sdk/build.rs | 13 +- core/crates/ployz/src/sdk/deploy.rs | 29 +- core/crates/ployz/src/sdk/github_build.rs | 71 ++--- core/crates/ployz/src/sdk/logs.rs | 46 +--- core/crates/ployz/src/sdk/metrics.rs | 24 +- core/crates/ployz/src/sdk/metrics_tests.rs | 4 +- core/crates/ployz/src/sdk/preparation.rs | 22 +- core/crates/ployz/src/sdk/prepare.rs | 9 +- core/crates/ployz/src/sdk/running.rs | 17 +- core/crates/ployz/src/sdk/store_call.rs | 20 +- core/crates/ployz/src/sdk/store_runner.rs | 19 +- .../ployz/src/sdk/store_runner_tests.rs | 4 +- core/crates/ployz/src/sealed_compat.rs | 19 +- core/crates/ployz/src/sealing_backfill.rs | 13 +- core/crates/ployz/src/ui/error.rs | 7 +- core/crates/ployz/src/ui/result.rs | 2 +- core/crates/ployzd/src/docker/stream.rs | 8 +- .../ployzd/src/machine/local_machine.rs | 21 +- core/crates/ployzd/src/machine_api/local.rs | 164 ++++------- .../src/machine_api/local_volume_copy.rs | 11 +- .../ployzd/src/management/build_grant.rs | 36 +-- .../ployzd/src/volume_plugin/capacity.rs | 15 +- .../ployzd/src/volume_plugin/inspect.rs | 11 +- 61 files changed, 775 insertions(+), 1292 deletions(-) diff --git a/core/crates/ployz-core/src/domain/data_loss.rs b/core/crates/ployz-core/src/domain/data_loss.rs index 94ab4835f8..9ac7cb24e0 100644 --- a/core/crates/ployz-core/src/domain/data_loss.rs +++ b/core/crates/ployz-core/src/domain/data_loss.rs @@ -115,12 +115,8 @@ impl UnconfirmedDataLoss { /// Refusal when execute-time Data Loss is not covered by the confirmation. #[must_use] pub fn into_rpc_error(self) -> RpcError { - RpcError { - code: RpcErrorCode::InvalidArgument, - message: self.to_string(), - details: serde_json::to_value(&self).expect("UnconfirmedDataLoss is JSON"), - cause: Vec::new(), - } + RpcError::invalid_argument(self.to_string()) + .with_details(serde_json::to_value(&self).expect("UnconfirmedDataLoss is JSON")) } /// Execute-time refusal payload, when `error` carries one. diff --git a/core/crates/ployz-core/src/rpc.rs b/core/crates/ployz-core/src/rpc.rs index c49c70f6c8..962063584e 100644 --- a/core/crates/ployz-core/src/rpc.rs +++ b/core/crates/ployz-core/src/rpc.rs @@ -2217,28 +2217,102 @@ pub struct RpcError { pub cause: Vec, } +/// The code of a refusal on the caller's access, such as a Server refusing a scope. +pub const FORBIDDEN_CODE: &str = "forbidden"; + +/// The code of a refusal that waits on a person's approval. +pub const APPROVAL_REQUIRED_CODE: &str = "approval_required"; + +macro_rules! named_constructors { + ($($(#[$meta:meta])* $name:ident => $code:ident,)*) => { + $( + $(#[$meta])* + #[must_use] + pub fn $name(message: impl Into) -> Self { + Self::new(RpcErrorCode::$code, message) + } + )* + }; +} + impl RpcError { - /// `error`'s own sentence as the message, and every source below it as a cause. + /// An error with no details and no causes. #[must_use] - pub fn caused(code: RpcErrorCode, error: &(dyn std::error::Error + 'static)) -> Self { + pub fn new(code: RpcErrorCode, message: impl Into) -> Self { Self { code, - message: error.to_string(), + message: message.into(), details: Value::Null, - cause: crate::error_chain::causes(error), + cause: Vec::new(), } } + named_constructors! { + invalid_argument => InvalidArgument, + not_found => NotFound, + ambiguous => Ambiguous, + unsupported => Unsupported, + unavailable => Unavailable, + conflict => Conflict, + internal => Internal, + unauthenticated => Unauthenticated, + confirmation_required => ConfirmationRequired, + } + + /// A refusal on the caller's access, coded [`FORBIDDEN_CODE`]. + #[must_use] + pub fn forbidden(message: impl Into) -> Self { + Self::new(RpcErrorCode::from(FORBIDDEN_CODE), message) + } + + /// A refusal that waits on a person's approval, coded [`APPROVAL_REQUIRED_CODE`]. + #[must_use] + pub fn approval_required(message: impl Into) -> Self { + Self::new(RpcErrorCode::from(APPROVAL_REQUIRED_CODE), message) + } + + /// `self` with `message` as its sentence. + #[must_use] + pub fn with_message(mut self, message: impl Into) -> Self { + self.message = message.into(); + self + } + + /// `self` with `details` as its body. + #[must_use] + pub fn with_details(mut self, details: Value) -> Self { + self.details = details; + self + } + + /// `self` with `error`'s own sentence and every source below it as its causes. + #[must_use] + pub fn with_cause(mut self, error: &(dyn std::error::Error + 'static)) -> Self { + self.cause = std::iter::once(error.to_string()) + .chain(crate::error_chain::causes(error)) + .collect(); + self + } + + /// `self` with `cause` as its causes, outermost first. + #[must_use] + pub fn with_causes(mut self, cause: Vec) -> Self { + self.cause = cause; + self + } + + /// `error`'s own sentence as the message, and every source below it as a cause. + #[must_use] + pub fn caused(code: RpcErrorCode, error: &(dyn std::error::Error + 'static)) -> Self { + Self::new(code, error.to_string()).with_causes(crate::error_chain::causes(error)) + } + /// An error the caller made because its target Machine didn't answer: the target /// sent nothing, so it may answer later. Read back by [`Self::target_unanswered`]. #[must_use] pub fn unanswered_target(code: RpcErrorCode, message: String) -> Self { - Self { - code, - message, - details: serde_json::json!({ "reason": TARGET_UNANSWERED_REASON }), - cause: Vec::new(), - } + Self::new(code, message) + .with_details(serde_json::json!({ "reason": TARGET_UNANSWERED_REASON })) } /// `self` from a call that failed while a Leave was removing `server`, so its copy @@ -2657,6 +2731,82 @@ mod rpc_error_wire { use super::*; use serde_json::json; + #[test] + fn constructors_serialize_like_the_literals_they_replace() { + let literal = |code: RpcErrorCode, details: Value, cause: Vec| RpcError { + code, + message: "m".into(), + details, + cause, + }; + let wire = |error: &RpcError| serde_json::to_string(error).unwrap(); + for (built, code) in [ + ( + RpcError::invalid_argument("m"), + RpcErrorCode::InvalidArgument, + ), + (RpcError::not_found("m"), RpcErrorCode::NotFound), + (RpcError::ambiguous("m"), RpcErrorCode::Ambiguous), + (RpcError::unsupported("m"), RpcErrorCode::Unsupported), + (RpcError::unavailable("m"), RpcErrorCode::Unavailable), + (RpcError::conflict("m"), RpcErrorCode::Conflict), + (RpcError::internal("m"), RpcErrorCode::Internal), + ( + RpcError::unauthenticated("m"), + RpcErrorCode::Unauthenticated, + ), + ( + RpcError::confirmation_required("m"), + RpcErrorCode::ConfirmationRequired, + ), + (RpcError::forbidden("m"), RpcErrorCode::from("forbidden")), + ( + RpcError::approval_required("m"), + RpcErrorCode::from("approval_required"), + ), + ( + RpcError::new(RpcErrorCode::Conflict, "m"), + RpcErrorCode::Conflict, + ), + ] { + let expected = literal(code, Value::Null, Vec::new()); + assert_eq!(built, expected); + assert_eq!(wire(&built), wire(&expected)); + } + assert_eq!( + wire(&RpcError::forbidden("m")), + r#"{"code":"forbidden","message":"m","details":null}"# + ); + + let detailed = RpcError::conflict("m") + .with_details(json!({ "k": 1 })) + .with_causes(vec!["a".into(), "b".into()]); + assert_eq!( + wire(&detailed), + wire(&literal( + RpcErrorCode::Conflict, + json!({ "k": 1 }), + vec!["a".into(), "b".into()] + )) + ); + assert_eq!( + RpcError::conflict("m").with_message("n"), + RpcError { + message: "n".into(), + ..literal(RpcErrorCode::Conflict, Value::Null, Vec::new()) + } + ); + } + + #[test] + fn with_cause_lists_the_error_then_its_sources() { + let inner = std::io::Error::other("disk"); + let outer = RpcError::caused(RpcErrorCode::Internal, &inner); + assert_eq!(outer.cause, Vec::::new()); + let chained = RpcError::internal("m").with_cause(&inner); + assert_eq!(chained.cause, vec!["disk".to_owned()]); + } + #[test] fn rpc_error_round_trips_cause() { let error = RpcError { diff --git a/core/crates/ployz-core/src/storage_capacity.rs b/core/crates/ployz-core/src/storage_capacity.rs index fec2b7016d..d58b5ca5fb 100644 --- a/core/crates/ployz-core/src/storage_capacity.rs +++ b/core/crates/ployz-core/src/storage_capacity.rs @@ -189,16 +189,12 @@ impl StorageCapacityError { ]), ); } - RpcError { - code: if matches!(self, Self::StorageCapacityUnknown { .. }) { - RpcErrorCode::Unavailable - } else { - RpcErrorCode::Conflict - }, - message: self.to_string(), - details, - cause: Vec::new(), - } + let code = if matches!(self, Self::StorageCapacityUnknown { .. }) { + RpcErrorCode::Unavailable + } else { + RpcErrorCode::Conflict + }; + RpcError::new(code, self.to_string()).with_details(details) } } diff --git a/core/crates/ployz-core/src/volume_copy.rs b/core/crates/ployz-core/src/volume_copy.rs index 8c47fc4969..0a3170b9e0 100644 --- a/core/crates/ployz-core/src/volume_copy.rs +++ b/core/crates/ployz-core/src/volume_copy.rs @@ -314,12 +314,8 @@ impl SetVolumeCopyRefused { | CopyRefusal::NoSource | CopyRefusal::Stale => RpcErrorCode::Conflict, }; - RpcError { - code, - message: message.into(), - details: serde_json::to_value(self).expect("a copy refusal is JSON serializable"), - cause: Vec::new(), - } + RpcError::new(code, message) + .with_details(serde_json::to_value(self).expect("a copy refusal is JSON serializable")) } } diff --git a/core/crates/ployz-core/src/volume_size.rs b/core/crates/ployz-core/src/volume_size.rs index 6d9c0725e6..fac639aa18 100644 --- a/core/crates/ployz-core/src/volume_size.rs +++ b/core/crates/ployz-core/src/volume_size.rs @@ -103,12 +103,8 @@ impl SetVolumeSizeRefused { | SizeRefusal::Shrink | SizeRefusal::BelowFloor => RpcErrorCode::Conflict, }; - RpcError { - code, - message: message.into(), - details: serde_json::to_value(self).expect("a size refusal is JSON serializable"), - cause: Vec::new(), - } + RpcError::new(code, message) + .with_details(serde_json::to_value(self).expect("a size refusal is JSON serializable")) } } diff --git a/core/crates/ployz-core/src/volume_switch.rs b/core/crates/ployz-core/src/volume_switch.rs index 9c47968147..e15534450d 100644 --- a/core/crates/ployz-core/src/volume_switch.rs +++ b/core/crates/ployz-core/src/volume_switch.rs @@ -457,12 +457,8 @@ impl SwitchError { /// An RPC error that carries this reason in `details`. #[must_use] pub fn rpc_error(self, message: impl Into) -> RpcError { - RpcError { - code: self.rpc_code(), - message: message.into(), - details: serde_json::to_value(self).expect("a switch reason is JSON serializable"), - cause: Vec::new(), - } + RpcError::new(self.rpc_code(), message) + .with_details(serde_json::to_value(self).expect("a switch reason is JSON serializable")) } const fn rpc_code(self) -> RpcErrorCode { diff --git a/core/crates/ployz-sdk/src/lib.rs b/core/crates/ployz-sdk/src/lib.rs index 184a17b24b..8261983cbe 100644 --- a/core/crates/ployz-sdk/src/lib.rs +++ b/core/crates/ployz-sdk/src/lib.rs @@ -12,7 +12,6 @@ use ployz::connect::diagnostics::{DialOutcome, DialTimings, should_emit}; use ployz::sdk; use ployz_core::{ DataLossConfirmation, ManagementClientLabel, Namespace, RemoveVolumesRequest, RpcError, - RpcErrorCode, }; /// One cancellable connection attempt. @@ -182,7 +181,7 @@ impl PendingConnection { let result = tokio::select! { biased; () = self.cancel.cancelled() => Err(sdk::CallError { - error: RpcError { code: RpcErrorCode::Unavailable, message: "connection cancelled".into(), details: serde_json::Value::Null, cause: Vec::new() }, + error: RpcError::unavailable("connection cancelled"), unanswered: true, target_unreached: false, }), @@ -313,6 +312,7 @@ pub struct RunningDeployHandle { #[cfg(test)] mod dial_diagnostic_tests { use super::*; + use ployz_core::RpcErrorCode; #[test] fn only_a_fired_deadline_is_timed_out() { @@ -1230,12 +1230,7 @@ pub(crate) fn whole_numbers(value: serde_json::Value) -> serde_json::Value { } fn invalid_argument(error: impl std::fmt::Display) -> Error { - rpc_to_napi(RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: serde_json::Value::Null, - cause: Vec::new(), - }) + rpc_to_napi(RpcError::invalid_argument(error.to_string())) } const RPC_ERROR_PREFIX: &str = "PLOYZ_RPC_ERROR:"; @@ -1283,15 +1278,8 @@ pub fn allocate_enrollment( let request = from_js(request).map_err(invalid_argument)?; let snapshot = from_js(snapshot).map_err(invalid_argument)?; let saved: Vec = from_js(saved).map_err(invalid_argument)?; - let assignment = - ployz_core::allocate_enrollment(&request, &snapshot, &saved).map_err(|error| { - rpc_to_napi(ployz_core::RpcError { - code: ployz_core::RpcErrorCode::Conflict, - message: error.to_string(), - details: serde_json::Value::Null, - cause: Vec::new(), - }) - })?; + let assignment = ployz_core::allocate_enrollment(&request, &snapshot, &saved) + .map_err(|error| rpc_to_napi(RpcError::conflict(error.to_string())))?; to_json(&assignment) } diff --git a/core/crates/ployz-store/src/build.rs b/core/crates/ployz-store/src/build.rs index ed569fd9f1..5ba5033baa 100644 --- a/core/crates/ployz-store/src/build.rs +++ b/core/crates/ployz-store/src/build.rs @@ -1072,14 +1072,8 @@ pub(crate) fn github_authorize( .into_iter() .find(|source| source.service == id.service) .ok_or_else(|| error::not_found("No GitHub build has this id", json!({})))?; - let refused = |message: &str| { - Err(ployz_core::RpcError { - code: ployz_core::RpcErrorCode::Unauthenticated, - message: message.to_owned(), - details: json!({}), - cause: Vec::new(), - }) - }; + let refused = + |message: &str| Err(RpcError::unauthenticated(message.to_owned()).with_details(json!({}))); if claims.repository_id != source.repository_id.to_string() { return refused("The token is for another repository"); } diff --git a/core/crates/ployz-store/src/deployment/admit.rs b/core/crates/ployz-store/src/deployment/admit.rs index 26abed63a2..270e578566 100644 --- a/core/crates/ployz-store/src/deployment/admit.rs +++ b/core/crates/ployz-store/src/deployment/admit.rs @@ -6,7 +6,7 @@ use ployz_core::authority::{DeployApproval, Signed}; use ployz_core::config::canonicalize_environment_intent; use ployz_core::config::{SavedEnvironmentIntent, SavedServiceIntent}; -use ployz_core::{Namespace, RpcError, RpcErrorCode, ServiceName}; +use ployz_core::{Namespace, RpcError, ServiceName}; use serde::{Deserialize, Serialize}; use serde_json::json; use ts_rs::TS; @@ -355,17 +355,13 @@ fn needs_cluster_domain( }) .collect::>() .join(" && "); - Err(RpcError { - code: RpcErrorCode::Unsupported, - message: format!( - "{} has generated domains ({}), which need the Cluster Domain Ployz Cloud \ + Err(RpcError::unsupported(format!( + "{} has generated domains ({}), which need the Cluster Domain Ployz Cloud \ reserves at a Deploy: deploy through Ployz Cloud, or remove them", - summary.name, - prefixes.join(", ") - ), - details: json!({ "domains": prefixes, "next": remove }), - cause: Vec::new(), - }) + summary.name, + prefixes.join(", ") + )) + .with_details(json!({ "domains": prefixes, "next": remove }))) } fn deploy( diff --git a/core/crates/ployz-store/src/error.rs b/core/crates/ployz-store/src/error.rs index 1a095421be..573b32a25e 100644 --- a/core/crates/ployz-store/src/error.rs +++ b/core/crates/ployz-store/src/error.rs @@ -4,12 +4,7 @@ use ployz_core::{RpcError, RpcErrorCode}; use serde_json::Value; fn error(code: RpcErrorCode, message: impl Into, details: Value) -> RpcError { - RpcError { - code, - message: message.into(), - details, - cause: Vec::new(), - } + RpcError::new(code, message).with_details(details) } pub(crate) fn invalid(message: impl Into, details: Value) -> RpcError { @@ -115,11 +110,19 @@ pub(crate) fn confirmation_required(message: impl Into, details: Value) } pub(crate) fn approval_required(message: impl Into, details: Value) -> RpcError { - error(RpcErrorCode::from("approval_required"), message, details) + error( + RpcErrorCode::from(ployz_core::APPROVAL_REQUIRED_CODE), + message, + details, + ) } pub(crate) fn forbidden(message: impl Into, details: Value) -> RpcError { - error(RpcErrorCode::from("forbidden"), message, details) + error( + RpcErrorCode::from(ployz_core::FORBIDDEN_CODE), + message, + details, + ) } /// Evidence a write needs is missing or incomplete, so it refuses rather than guess. diff --git a/core/crates/ployz-testkit/src/fake_fleet.rs b/core/crates/ployz-testkit/src/fake_fleet.rs index 4bd13d8d35..8941547a92 100644 --- a/core/crates/ployz-testkit/src/fake_fleet.rs +++ b/core/crates/ployz-testkit/src/fake_fleet.rs @@ -35,7 +35,7 @@ use ployz_core::{ MembershipObservation, NAMESPACE_LABEL, Namespace, OpaquePayload, PROTOCOL_MAJOR, PrivateAccessSet, ProvisionedCopy, ProvisionedVolumeMaximumBytes, QualifiedService, RUNTIME_WATCH_MESSAGE_SIZE_LIMIT, ResolvedServiceSpec, RotationEdge, RoutedAddresses, - RoutesRead, RoutingRequest, RpcError, RpcErrorCode, RpcRequestBody, RpcResponse, RunTarget, + RoutesRead, RoutingRequest, RpcError, RpcRequestBody, RpcResponse, RunTarget, RuntimeWatchFrame, ServiceId, ServiceName, SetPrivateAccessRequest, SlotId, SlotRole, StorageBacking, StorageCapacity, VolumeCopy, VolumeCopyView, VolumeInventory, WireGuardPublicKey, WriterEpoch, encode_runtime_watch_frame, @@ -1453,13 +1453,7 @@ fn physical(place: &VolumePlace) -> DockerVolumeName { } fn not_found(container_id: &ContainerId) -> RpcResponse { - RpcError { - code: RpcErrorCode::NotFound, - message: format!("no Container {container_id}"), - details: Value::Null, - cause: Vec::new(), - } - .into() + RpcError::not_found(format!("no Container {container_id}")).into() } fn unimplemented(command: &str) -> Status { diff --git a/core/crates/ployz/src/approval.rs b/core/crates/ployz/src/approval.rs index dbee125805..896813882c 100644 --- a/core/crates/ployz/src/approval.rs +++ b/core/crates/ployz/src/approval.rs @@ -18,9 +18,6 @@ use crate::failure::Failure; const POLL: Duration = Duration::from_secs(1); -/// The code of Cloud's refusal that waits for a person to approve the call. -pub(crate) const APPROVAL_REQUIRED: &str = "approval_required"; - #[derive(Debug, Deserialize)] pub(crate) struct Asked { #[serde(rename = "approval_id")] @@ -212,7 +209,7 @@ impl Verb { impl Asked { pub(crate) fn of(error: &RpcError) -> Option { - if error.code.as_str() != APPROVAL_REQUIRED { + if error.code.as_str() != ployz_core::APPROVAL_REQUIRED_CODE { return None; } let asked: Self = serde_json::from_value(error.details.clone()).ok()?; diff --git a/core/crates/ployz/src/cloud_account.rs b/core/crates/ployz/src/cloud_account.rs index 06780972f9..07994df68e 100644 --- a/core/crates/ployz/src/cloud_account.rs +++ b/core/crates/ployz/src/cloud_account.rs @@ -878,16 +878,11 @@ async fn follow_operation_until( follow_run(credential, path, doing, Some(deadline)) .await? .ok_or_else(|| { - StoreCallError::Refused(RpcError { - code: ployz_core::RpcErrorCode::Unavailable, - message: format!( - "Stopped waiting after {} minutes. {doing} keeps running in Ployz Cloud and \ + StoreCallError::Refused(RpcError::unavailable(format!( + "Stopped waiting after {} minutes. {doing} keeps running in Ployz Cloud and \ finishes on its own; this command no longer reports it.", - crate::handlers::mcp::CALL_DEADLINE.as_secs() / 60 - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }) + crate::handlers::mcp::CALL_DEADLINE.as_secs() / 60 + ))) }) } @@ -1171,15 +1166,14 @@ pub(crate) async fn follow_removal( { // Any step after the reset was sent may be the one that ran out: the Server may // have reset and Cloud dropped it, or neither. - Err(StoreCallError::Refused(RpcError { - code: ployz_core::RpcErrorCode::Unavailable, - message: format!( + Err(StoreCallError::Refused( + RpcError::unavailable(format!( "Cloud's removal of Server {name} didn't finish: it ran out of retries. If \ the Server is gone but `ployz server ls` still lists it, forget it." - ), - details: serde_json::json!({ "next": "ployz server ls" }), - cause: vec![refused.message], - })) + )) + .with_details(serde_json::json!({ "next": "ployz server ls" })) + .with_causes(vec![refused.message]), + )) } Err(error) => Err(error), } @@ -1205,16 +1199,11 @@ async fn removal_progress(verb: &str, name: &MachineName, mut say: impl FnMut(St /// Stopped watching a removal that is still running. It may yet fail unfinished, as /// [`follow_removal`] says when it does. No `next`: nothing is to run while it runs. fn removal_unsettled(name: &MachineName) -> RpcError { - RpcError { - code: ployz_core::RpcErrorCode::Unavailable, - message: format!( - "Cloud is still removing Server {name}; it finishes on its own. \ + RpcError::unavailable(format!( + "Cloud is still removing Server {name}; it finishes on its own. \ The dashboard's Servers page shows when it's done. If it fails and the Server \ is gone but `ployz server ls` still lists it, forget it." - ), - details: serde_json::Value::Null, - cause: Vec::new(), - } + )) } /// Call `/api/cli/`, answered as its reply or as Cloud's refusal, verbatim. @@ -1316,14 +1305,9 @@ pub(crate) async fn upload( /// The refusal of a source too large to upload: `why`, then how to deploy it anyway. fn too_big(why: &str) -> RpcError { - RpcError { - code: ployz_core::RpcErrorCode::InvalidArgument, - message: format!( - "{why} Deploy from GitHub instead, or remove large files from the directory. In a Git checkout, ployz up leaves out files Git ignores, so add ones such as node_modules to .gitignore." - ), - details: serde_json::Value::Null, - cause: Vec::new(), - } + RpcError::invalid_argument(format!( + "{why} Deploy from GitHub instead, or remove large files from the directory. In a Git checkout, ployz up leaves out files Git ignores, so add ones such as node_modules to .gitignore." + )) } /// Call `/api/cli/`. A 404 on a read means Cloud doesn't offer the CLI surface, @@ -1454,12 +1438,7 @@ pub(crate) fn public_error(body: &[u8]) -> Option { "RATE_LIMITED" => ployz_core::RpcErrorCode::Unavailable, _ => ployz_core::RpcErrorCode::Internal, }; - Some(RpcError { - code, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - }) + Some(RpcError::new(code, message)) } async fn send( diff --git a/core/crates/ployz/src/cluster.rs b/core/crates/ployz/src/cluster.rs index f1e70eccaf..e63324b10c 100644 --- a/core/crates/ployz/src/cluster.rs +++ b/core/crates/ployz/src/cluster.rs @@ -960,12 +960,9 @@ impl Client { .await .map_err(|status| TransportError::from_stream_status(status).to_rpc_error())? else { - return Err(RpcError { - code: RpcErrorCode::Unavailable, - message: "Runtime Watch ended before its first frame".into(), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::unavailable( + "Runtime Watch ended before its first frame", + )); }; decode_runtime_watch_frame(&frame) .map_err(|error| crate::ui::rpc_error(RpcErrorCode::Internal, &error)) @@ -1315,14 +1312,9 @@ impl Client { .map_err(RpcError::from)? .machine_id; if selected == current && machines.len() > 1 { - return Err(RpcError { - code: RpcErrorCode::InvalidArgument, - message: - "the current entry Machine cannot be removed while another Machine is visible" - .into(), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::invalid_argument( + "the current entry Machine cannot be removed while another Machine is visible", + )); } let hold = refuse_last_managed(self, &machines, selected).await?; if hold == CloudHold::Last && remover == Remover::Operator { @@ -1409,17 +1401,13 @@ impl Client { Err(error) => { let error = rpc_error(error); let unlisted = error.server_unlisted(&machine.machine.id); - let unchecked = RpcError { - code: RpcErrorCode::Unavailable, - // Often only the entry was busy, as when several forgets share it. - message: format!( - "Couldn't check whether Server {} holds Volume copies. No changes made. \ - Run the command again, one Server at a time if you're forgetting several.", - machine.machine.name - ), - details: Value::Null, - cause: std::iter::once(error.message).chain(error.cause).collect(), - }; + // Often only the entry was busy, as when several forgets share it. + let unchecked = RpcError::unavailable(format!( + "Couldn't check whether Server {} holds Volume copies. No changes made. \ + Run the command again, one Server at a time if you're forgetting several.", + machine.machine.name + )) + .with_causes(std::iter::once(error.message).chain(error.cause).collect()); if unlisted { Ok(HeldCopies::Gone(unchecked)) } else { @@ -1782,12 +1770,7 @@ impl Client { machine_id, error: ContainerOperationFailure { container_id, - error: RpcError { - code: RpcErrorCode::Unavailable, - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - }, + error: RpcError::unavailable(error.to_string()), }, }); } @@ -1932,18 +1915,13 @@ pub(crate) enum DeadConfirmed { } fn stopped_answering(machine: &Machine) -> RpcError { - RpcError { - code: RpcErrorCode::ConfirmationRequired, - message: format!( + RpcError::confirmation_required(format!( "Server {} stopped answering after its name was typed. Forgetting it leaves any Volume copies on it behind, so it needs dead typed. No changes made.", machine.name - ), - details: json!({ + )).with_details(json!({ "server": { "id": machine.id, "name": machine.name }, "answered": false, - }), - cause: Vec::new(), - } + })) } #[derive(Debug)] @@ -1980,30 +1958,20 @@ pub(crate) fn copies_left_behind<'a>( machine: &MachineName, volumes: impl IntoIterator, ) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!( - "Server {machine} answers and holds copies of Volumes {}; forgetting it leaves them \ + RpcError::conflict(format!( + "Server {machine} answers and holds copies of Volumes {}; forgetting it leaves them \ behind. No changes made. Remove it with ployz server rm so its reset demotes them.", - volumes.into_iter().collect::>().join(", ") - ), - details: Value::Null, - cause: Vec::new(), - } + volumes.into_iter().collect::>().join(", ") + )) } /// The last Machine is Cloud's: anyone else removing it would leave Cloud paired /// with a Cluster that no longer exists. fn cloud_holds_last(machine: MachineId) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!( - "Machine {machine} is the last Machine and Cloud manages it; only Cloud's removal \ + RpcError::conflict(format!( + "Machine {machine} is the last Machine and Cloud manages it; only Cloud's removal \ lets go of it too. No changes made." - ), - details: Value::Null, - cause: Vec::new(), - } + )) } /// Refuse removing the last Machine while a Management Client other than Cloud still @@ -2030,17 +1998,14 @@ pub(crate) async fn refuse_last_managed( Some(TARGET_RPC_TIMEOUT), ) .await - .map_err(|error| RpcError { - code: RpcErrorCode::Unavailable, - message: crate::ui::then_sentence( + .map_err(|error| { + RpcError::unavailable(crate::ui::then_sentence( &format!( "cannot read who manages Machine {selected}, the last Machine: {}", crate::ui::row(&error) ), "No changes made.", - ), - details: Value::Null, - cause: Vec::new(), + )) })? .management_clients; let cloud = holders.iter().any(|label| label.as_str() == "cloud"); @@ -2057,16 +2022,11 @@ pub(crate) async fn refuse_last_managed( Some((last, [])) => last.clone(), Some((last, rest)) => format!("{} and {last}", rest.join(", ")), }; - Err(RpcError { - code: RpcErrorCode::Conflict, - message: format!( - "this is the last Machine in the Cluster and it is still managed by {who}; \ + Err(RpcError::conflict(format!( + "this is the last Machine in the Cluster and it is still managed by {who}; \ removing it would leave {who} managing a Cluster that no longer exists. \ No changes made. Disconnect {who} from this Machine first." - ), - details: Value::Null, - cause: Vec::new(), - }) + ))) } /// Resolve a Machine Target without hiding Name Ambiguity in the visible observations. @@ -2079,28 +2039,21 @@ pub(crate) fn visible_machine<'list>( ) -> Result<&'list MachineObservation, RpcError> { let selected = match machine.resolve(machines.iter().map(|entry| &entry.machine)) { NameMatches::None => { - return Err(RpcError { - code: RpcErrorCode::NotFound, - message: format!("Server {} was not found", machine.as_str().escape_debug()), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::not_found(format!( + "Server {} was not found", + machine.as_str().escape_debug() + ))); } matches @ NameMatches::Ambiguous { .. } => { - return Err(RpcError { - code: RpcErrorCode::Ambiguous, - message: format!( - "Server name {} is ambiguous: {}", - machine.as_str().escape_debug(), - matches - .iter() - .map(|row| row.id.as_str()) - .collect::>() - .join(", ") - ), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::ambiguous(format!( + "Server name {} is ambiguous: {}", + machine.as_str().escape_debug(), + matches + .iter() + .map(|row| row.id.as_str()) + .collect::>() + .join(", ") + ))); } NameMatches::One(row) => row.id, }; @@ -2358,14 +2311,13 @@ pub(crate) struct LeftNotForgotten { impl LeftNotForgotten { fn carried_by(error: RpcError, reset_warning: Option) -> RpcError { - RpcError { - details: serde_json::to_value(Self { + error.with_details( + serde_json::to_value(Self { left: true, reset_warning, }) .expect("LeftNotForgotten is JSON"), - ..error - } + ) } /// The Leave reply a failed removal carries, when it carries one. @@ -2399,9 +2351,9 @@ async fn data_loss_on_machine( } let volumes = list_volumes_on_machine(client.clone(), selected) .await - .map_err(|failure| RpcError { - message: format!("Server {name}: {}", crate::ui::row(&failure.error)), - ..failure.error + .map_err(|failure| { + let message = format!("Server {name}: {}", crate::ui::row(&failure.error)); + failure.error.with_message(message) })?; Ok(ObservedDataLoss { data_loss: volumes_lost(volumes.value), @@ -2422,12 +2374,7 @@ fn volumes_lost(inventory: VolumeInventory) -> Vec { } fn server_did_not_respond(name: &MachineName) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: format!("Server {name} did not respond"), - details: Value::Null, - cause: Vec::new(), - } + RpcError::unavailable(format!("Server {name} did not respond")) } /// Machines whose storage observation failed (`failures`) or did not answer in @@ -2500,26 +2447,16 @@ fn validate_volume_inventory( .chain(inventory.failures.iter().map(|failure| &failure.id)) { if id.machine_id != machine_id { - return Err(RpcError { - code: RpcErrorCode::Internal, - message: format!( - "targeted ListVolumes response from Machine {machine_id} contained Docker Volume '{}' for Machine {}", - id.name, id.machine_id - ), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::internal(format!( + "targeted ListVolumes response from Machine {machine_id} contained Docker Volume '{}' for Machine {}", + id.name, id.machine_id + ))); } if !names.insert(&id.name) { - return Err(RpcError { - code: RpcErrorCode::Internal, - message: format!( - "targeted ListVolumes response from Machine {machine_id} repeated Docker Volume '{}'", - id.name - ), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::internal(format!( + "targeted ListVolumes response from Machine {machine_id} repeated Docker Volume '{}'", + id.name + ))); } } Ok(inventory) @@ -2555,10 +2492,7 @@ fn unanswered_kept(error: ConnectError) -> RpcError { } let unreached = error.target_unreached(); let error = rpc_error(error); - let kept = RpcError { - cause: error.cause, - ..RpcError::unanswered_target(error.code, error.message) - }; + let kept = RpcError::unanswered_target(error.code, error.message).with_causes(error.cause); match unreached { true => kept.undialed_target(), false => kept, @@ -2629,14 +2563,9 @@ async fn change_container_rpc( fn admitted(started: UserStarted) -> Result<(), RpcError> { match started { UserStarted::In | UserStarted::Running => Ok(()), - UserStarted::NotIn(health) => Err(RpcError { - code: RpcErrorCode::Unavailable, - message: "The Container started but did not pass its healthcheck, so proxies do not route to it.".into(), - details: health + UserStarted::NotIn(health) => Err(RpcError::unavailable("The Container started but did not pass its healthcheck, so proxies do not route to it.").with_details(health .and_then(ployz_core::HealthVerdict::into_failure) - .map_or(Value::Null, |failure| json!(failure)), - cause: Vec::new(), - }), + .map_or(Value::Null, |failure| json!(failure)))), } } diff --git a/core/crates/ployz/src/connect.rs b/core/crates/ployz/src/connect.rs index 1c07a5b1ef..18ff9e2c40 100644 --- a/core/crates/ployz/src/connect.rs +++ b/core/crates/ployz/src/connect.rs @@ -770,43 +770,26 @@ pub(crate) fn rpc_error(error: ConnectError) -> RpcError { { rpc_error(*error) } - ConnectError::ClientCleared => RpcError { - code: RpcErrorCode::Unauthenticated, - message: "Machine confirmed this Management Client was cleared".into(), - details: json!({ "management_client": "cleared" }), - cause: Vec::new(), - }, + ConnectError::ClientCleared => { + RpcError::unauthenticated("Machine confirmed this Management Client was cleared") + .with_details(json!({ "management_client": "cleared" })) + } ConnectError::Remote(error) => error, ConnectError::Rpc(error) => error.to_rpc_error(), - error @ ConnectError::OlderMachine => RpcError { - code: RpcErrorCode::Unsupported, - message: error.to_string(), - details: json!({ "management_protocol": "older" }), - cause: Vec::new(), - }, + error @ ConnectError::OlderMachine => RpcError::unsupported(error.to_string()) + .with_details(json!({ "management_protocol": "older" })), // A refused entry: the call never went out, whatever answered the dial. error @ (ConnectError::IdentityMismatch { .. } | ConnectError::ClusterMismatch { .. }) => { - RpcError { - code: RpcErrorCode::Unauthenticated, - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - } - .unreached() + RpcError::unauthenticated(error.to_string()).unreached() } error @ (ConnectError::ClusterUnverified { .. } | ConnectError::PredatesSealing { .. } | ConnectError::ClientRefused - | ConnectError::TunnelNeedsFull) => RpcError { - code: RpcErrorCode::Unauthenticated, - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - }, - error @ ConnectError::Codec(CodecError::DecodeJson(_)) => RpcError { - details: json!({ "answer": UNREADABLE_ANSWER }), - ..crate::ui::rpc_error(RpcErrorCode::Internal, &error) - }, + | ConnectError::TunnelNeedsFull) => RpcError::unauthenticated(error.to_string()), + error @ ConnectError::Codec(CodecError::DecodeJson(_)) => { + crate::ui::rpc_error(RpcErrorCode::Internal, &error) + .with_details(json!({ "answer": UNREADABLE_ANSWER })) + } error @ (ConnectError::Attempt(_) | ConnectError::Exhausted(_) | ConnectError::EntryNotReady @@ -1743,16 +1726,13 @@ impl TransportError { #[must_use] pub fn to_rpc_error(&self) -> RpcError { - RpcError { - code: rpc_error_code(self.code), - message: self.message.clone(), - details: if self.session_closed { + RpcError::new(rpc_error_code(self.code), self.message.clone()) + .with_details(if self.session_closed { json!({ "reason": SESSION_CLOSED_REASON }) } else { self.details.clone() - }, - cause: ployz_core::error_chain::causes(self), - } + }) + .with_causes(ployz_core::error_chain::causes(self)) } } @@ -1868,13 +1848,10 @@ impl SshFailure { } } -/// The code of a refusal on the caller's access, such as a Server refusing a scope. -pub(crate) const FORBIDDEN: &str = "forbidden"; - /// Whether a Machine refused `capability` because Device enforcement is on, as it /// refuses an Operator's `Build` and `MintBuildGrant`. pub(crate) fn refused_by_enforcement(error: &RpcError, capability: &str) -> bool { - error.code.as_str() == FORBIDDEN + error.code.as_str() == ployz_core::FORBIDDEN_CODE && error.details.get("capability").and_then(Value::as_str) == Some(capability) } @@ -1887,7 +1864,7 @@ fn rpc_error_code(code: tonic::Code) -> RpcErrorCode { tonic::Code::Unimplemented => RpcErrorCode::Unsupported, tonic::Code::Unauthenticated => RpcErrorCode::Unauthenticated, // The spelling Cloud's refusals already use; a reader that doesn't know it keeps it. - tonic::Code::PermissionDenied => RpcErrorCode::from(FORBIDDEN), + tonic::Code::PermissionDenied => RpcErrorCode::from(ployz_core::FORBIDDEN_CODE), tonic::Code::Ok | tonic::Code::Cancelled | tonic::Code::Unknown diff --git a/core/crates/ployz/src/deploy.rs b/core/crates/ployz/src/deploy.rs index dd87a650ab..bec969ac8a 100644 --- a/core/crates/ployz/src/deploy.rs +++ b/core/crates/ployz/src/deploy.rs @@ -7,8 +7,8 @@ use ployz_core::{ BridgeEndpointCapacity, ContainerObservation, ContainerRuntimeObservation, DockerVolume, DockerVolumeId, DockerVolumeName, IngressHost, MachineFailure, MachineId, MachineName, MachineObservation, MembershipObservation, Namespace, PartialResult, PlacementConstraint, - ProvisionedVolumeMaximumBytes, QualifiedService, RowStanding, RpcError, RpcErrorCode, - ServiceName, ServiceObservation, VolumeInventory, VolumeObservationFailure, derive_services, + ProvisionedVolumeMaximumBytes, QualifiedService, RowStanding, RpcError, ServiceName, + ServiceObservation, VolumeInventory, VolumeObservationFailure, derive_services, }; use thiserror::Error; @@ -88,11 +88,12 @@ impl VolumeSnapshot { named_failures.extend(success.value.failures); } Self::try_from_parts(observations, named_failures, machine_failures, omissions).map_err( - |error| RpcError { - code: RpcErrorCode::Internal, - message: format!("invalid Docker Volume snapshot: {}", crate::ui::row(&error)), - details: error.details, - cause: Vec::new(), + |error| { + RpcError::internal(format!( + "invalid Docker Volume snapshot: {}", + crate::ui::row(&error) + )) + .with_details(error.details) }, ) } @@ -135,12 +136,7 @@ impl VolumeSnapshot { } fn validate(&self) -> Result<(), RpcError> { - let invalid = |message| RpcError { - code: RpcErrorCode::InvalidArgument, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - }; + let invalid = |message| RpcError::invalid_argument(message); let mut ids = BTreeSet::new(); for volume in &self.observations { if !ids.insert(&volume.id) { @@ -993,18 +989,13 @@ impl PlanError { error @ Self::NoWriter { .. } => { ployz_core::SwitchError::NoWriter.rpc_error(error.to_string()) } - error @ Self::VolumeServerTakesNoServices { .. } => RpcError { - code: RpcErrorCode::Conflict, - message: error.to_string(), - details: serde_json::json!({ "reason": "volume_server_takes_no_services" }), - cause: Vec::new(), - }, - error @ (Self::CopiesUnknown { .. } | Self::VolumeHolderUnknown { .. }) => RpcError { - code: RpcErrorCode::Unavailable, - message: error.to_string(), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error @ Self::VolumeServerTakesNoServices { .. } => RpcError::conflict( + error.to_string(), + ) + .with_details(serde_json::json!({ "reason": "volume_server_takes_no_services" })), + error @ (Self::CopiesUnknown { .. } | Self::VolumeHolderUnknown { .. }) => { + RpcError::unavailable(error.to_string()) + } error @ (Self::ConflictingHostPublications { .. } | Self::HostPortConflict { .. } | Self::CapacityUnknown @@ -1023,16 +1014,13 @@ impl PlanError { | Self::HostnameConflict { .. } | Self::RemovalInTarget { .. } | Self::RemovalNotSelected { .. } - | Self::RemovalInReservedNamespace { .. }) => RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: if error.same_again() { + | Self::RemovalInReservedNamespace { .. }) => { + RpcError::invalid_argument(error.to_string()).with_details(if error.same_again() { serde_json::json!({"same_again": true}) } else { serde_json::Value::Null - }, - cause: Vec::new(), - }, + }) + } } } diff --git a/core/crates/ployz/src/deploy/cleanup.rs b/core/crates/ployz/src/deploy/cleanup.rs index 9d9a88c14a..ca6def1307 100644 --- a/core/crates/ployz/src/deploy/cleanup.rs +++ b/core/crates/ployz/src/deploy/cleanup.rs @@ -8,12 +8,11 @@ use ployz_core::{ ResolvedServiceSpec, RoutesRead, RpcError, RpcErrorCode, RunRotation, RunTarget, StopOptions, Unacked, }; -use serde_json::Value; use tokio::time::Instant; use tokio_util::sync::CancellationToken; use super::MachineOperations; -use crate::connect::{FORBIDDEN, TARGET_RPC_TIMEOUT, in_rpc_timeout, stop_rpc_timeout}; +use crate::connect::{TARGET_RPC_TIMEOUT, in_rpc_timeout, stop_rpc_timeout}; use crate::ui::{then_sentence, with_reason}; pub(crate) const ACK_POLL: Duration = Duration::from_millis(200); @@ -441,7 +440,7 @@ pub(crate) async fn user_stop( let step = hold(client, ©, run.clone(), cancellation).await; if copy.rotation == CopyRotation::In { let failed = match step { - Ok(Step::Cancelled) => Some(unavailable( + Ok(Step::Cancelled) => Some(RpcError::unavailable( "the wait for its proxies was cancelled".to_owned(), )), // The ask may have landed, so the copy is restored as if Held. @@ -525,32 +524,30 @@ pub(crate) async fn user_restart( let stopped = match hold(client, ©, run.clone(), cancellation).await { Ok(Step::Refused(refusal)) => return Err(refusal.into_error()), Ok(Step::Done) => stopped(client, &held, spec, stop, queued).await, - Ok(Step::NotConfirmed(unconfirmed)) => Err(unavailable(format!( + Ok(Step::NotConfirmed(unconfirmed)) => Err(RpcError::unavailable(format!( "Container {} was not restarted: the proxies on {} did not confirm they stopped routing it.", copy.container_id, listed(&unconfirmed.machines) ))), - Ok(Step::Cancelled) => Err(unavailable(format!( + Ok(Step::Cancelled) => Err(RpcError::unavailable(format!( "Container {} was not restarted: the wait for its proxies was cancelled.", copy.container_id ))), - Err(error) if error.code.as_str() == FORBIDDEN => { - return Err(RpcError { - message: with_reason( - format_args!("Container {} was not restarted", copy.container_id), - &error.message, - ), - ..error - }); + Err(error) if error.code.as_str() == ployz_core::FORBIDDEN_CODE => { + let message = with_reason( + format_args!("Container {} was not restarted", copy.container_id), + &error.message, + ); + return Err(error.with_message(message)); } // The ask may have landed, so the copy is restored as if Held. - Err(error) => Err(RpcError { - message: with_reason( + Err(error) => { + let message = with_reason( format_args!("Container {} was not restarted", copy.container_id), &error.message, - ), - ..error - }), + ); + Err(error.with_message(message)) + } }; let brought = bring_in(client, &held, spec, Some(run)).await; if let Err(mut error) = stopped { @@ -595,16 +592,11 @@ pub(crate) async fn user_remove( /// The inspect saw Docker answer for `container_id` while its Machine holds no Resolved /// Service Spec under that ID. The remove asks for nothing, so the Container stays. fn kept_without_spec(container_id: &ContainerId, inspected: RpcError) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!( + RpcError::conflict(format!( "Container {container_id} was not removed: Docker on its Server has it, and the Server holds no Resolved Service Spec under its ID." - ), - details: serde_json::json!({ "reason": ContainerNotFoundReason::NoResolvedSpec }), - cause: std::iter::once(inspected.message) + )).with_details(serde_json::json!({ "reason": ContainerNotFoundReason::NoResolvedSpec })).with_causes(std::iter::once(inspected.message) .chain(inspected.cause) - .collect(), - } + .collect()) } async fn observe( @@ -631,16 +623,11 @@ fn never_in(observed: &ContainerObservation) -> bool { } fn not_admitted(copy: &Copy) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!( - "Container {} is left {} and not routed: no Deploy brought it In. Deploy the Service again to route it.", - copy.container_id, - rotation_name(©.rotation) - ), - details: Value::Null, - cause: Vec::new(), - } + RpcError::conflict(format!( + "Container {} is left {} and not routed: no Deploy brought it In. Deploy the Service again to route it.", + copy.container_id, + rotation_name(©.rotation) + )) } /// Hooks stay Pending for life, and neither Ingress door is ever routed. @@ -757,7 +744,7 @@ async fn not_stopped( format_args!("Container {} was not stopped", copy.container_id), &error.message, ); - if error.code.as_str() == FORBIDDEN { + if error.code.as_str() == ployz_core::FORBIDDEN_CODE { return error; } let held = Copy { @@ -778,12 +765,12 @@ fn left_running(copy: &Copy, rotation: &str, step: Step) -> Result<(), RpcError> match step { Step::Done => Ok(()), Step::Refused(refusal) => Err(refusal.into_error()), - Step::NotConfirmed(unconfirmed) => Err(unavailable(format!( + Step::NotConfirmed(unconfirmed) => Err(RpcError::unavailable(format!( "Container {} is left running and {rotation}: the proxies on {} did not confirm they stopped routing it.", copy.container_id, listed(&unconfirmed.machines) ))), - Step::Cancelled => Err(unavailable(format!( + Step::Cancelled => Err(RpcError::unavailable(format!( "Container {} is left running and {rotation}: the wait for its proxies was cancelled.", copy.container_id ))), @@ -791,24 +778,10 @@ fn left_running(copy: &Copy, rotation: &str, step: Step) -> Result<(), RpcError> } fn deploy_again(copy: &Copy) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!( - "Container {} is out of rotation and cannot start again. Deploy the Service again.", - copy.container_id - ), - details: Value::Null, - cause: Vec::new(), - } -} - -fn unavailable(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message, - details: Value::Null, - cause: Vec::new(), - } + RpcError::conflict(format!( + "Container {} is out of rotation and cannot start again. Deploy the Service again.", + copy.container_id + )) } fn listed(machines: &[MachineId]) -> String { diff --git a/core/crates/ployz/src/deploy/exec/mod.rs b/core/crates/ployz/src/deploy/exec/mod.rs index 1ec4bc7275..f4ddd7dec6 100644 --- a/core/crates/ployz/src/deploy/exec/mod.rs +++ b/core/crates/ployz/src/deploy/exec/mod.rs @@ -345,12 +345,9 @@ impl MachineOperations for Client { .map(|machine| &machine.membership), ) }) { - return Err(RpcError { - code: RpcErrorCode::Unavailable, - message: format!("container observation omitted {machine_id}"), - details: serde_json::Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::unavailable(format!( + "container observation omitted {machine_id}" + ))); } let copies = listed .successes @@ -1106,7 +1103,7 @@ pub(super) async fn execute_operation_sequence( last, machine_error( MachineAction::ReadRoutes, - unavailable(rows.not_confirmed(&machines)), + RpcError::unavailable(rows.not_confirmed(&machines)), ), Event::PublishUnconfirmed(group), ); @@ -1682,7 +1679,7 @@ async fn publish_group( Ok(cleanup::Wait::TimedOut(machines) | cleanup::Wait::Gapped(machines)) => { let error = machine_error( MachineAction::ReadRoutes, - unavailable(rows.not_confirmed(&machines)), + RpcError::unavailable(rows.not_confirmed(&machines)), ); (Some(Left::NotConfirmed(machines)), Some(error)) } @@ -2273,7 +2270,7 @@ impl<'a> Rows<'a> { Ok(cleanup::Step::NotConfirmed(machines)) => { let error = machine_error( MachineAction::StopContainer, - unavailable(self.not_confirmed(&machines)), + RpcError::unavailable(self.not_confirmed(&machines)), ); (Some(Left::NotConfirmed(machines)), Some(error)) } @@ -2527,12 +2524,7 @@ impl<'a> Rows<'a> { wait = ?STORED_GENS_WAIT, "the entry Machine's store didn't answer in time; new gens follow the live answers only" ); - return Ok(Err(RpcError { - code: RpcErrorCode::Unavailable, - message: format!("the store didn't answer in {STORED_GENS_WAIT:?}"), - details: serde_json::Value::Null, - cause: Vec::new(), - })); + return Ok(Err(RpcError::unavailable(format!("the store didn't answer in {STORED_GENS_WAIT:?}")))); } }; Ok(Ok(containers @@ -2557,7 +2549,7 @@ impl<'a> Rows<'a> { Ok(cleanup::Step::Refused(refusal)) => Err(machine_error(action, refusal.into_error())), Ok(cleanup::Step::NotConfirmed(machines)) => Err(machine_error( action, - unavailable(self.not_confirmed(&machines)), + RpcError::unavailable(self.not_confirmed(&machines)), )), Ok(cleanup::Step::Cancelled) => Err(ExecutionError::Cancelled), Err(error) => Err(machine_error(action, error)), @@ -3302,7 +3294,7 @@ async fn run_new( // whatever the slot holds. AnswerWhen::Settled => tokio::select! { answered = ensured => answered, - () = cancellation.cancelled() => return Err(unavailable(format!( + () = cancellation.cancelled() => return Err(RpcError::unavailable(format!( "the Deploy was cancelled while Machine {machine_id} waited on health" ))), }, @@ -3312,24 +3304,10 @@ async fn run_new( progress.set_display_name(index, observed.display_name.clone()); Ok((observed.container_id, health)) } - (ContainerNow::Absent { .. }, _) => Err(RpcError { - code: RpcErrorCode::Internal, - message: format!( - "the Machine started no Container for slot {}", - slot.creation_key.as_str() - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }), - } -} - -fn unavailable(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message, - details: serde_json::Value::Null, - cause: Vec::new(), + (ContainerNow::Absent { .. }, _) => Err(RpcError::internal(format!( + "the Machine started no Container for slot {}", + slot.creation_key.as_str() + ))), } } @@ -3626,7 +3604,7 @@ async fn remove_container( Ok(cleanup::Step::NotConfirmed(machines)) => { let error = machine_error( MachineAction::RemoveContainer, - unavailable(rows.not_confirmed(&machines)), + RpcError::unavailable(rows.not_confirmed(&machines)), ); rows.leave_running(progress, ©, Left::NotConfirmed(machines)); Err(error.into()) @@ -3678,7 +3656,7 @@ async fn stop_old_copy( Ok(cleanup::Step::NotConfirmed(machines)) => { let error = machine_error( MachineAction::StopContainer, - unavailable(rows.not_confirmed(&machines)), + RpcError::unavailable(rows.not_confirmed(&machines)), ); (Left::NotConfirmed(machines), error) } @@ -4083,18 +4061,16 @@ struct Numbered { /// A new copy cannot start while `silent` may run copies the store could not number. fn unfloored(silent: &Unanswered, unread: &RpcError) -> ExecutionError { - let error = RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "Machine '{}' did not answer and the entry Machine's store could not be read, \ + let error = RpcError::unavailable(format!( + "Machine '{}' did not answer and the entry Machine's store could not be read, \ so a new copy might not replace that Machine's copies", - silent.name - ), - details: serde_json::Value::Null, - cause: std::iter::once(unread.message.clone()) + silent.name + )) + .with_causes( + std::iter::once(unread.message.clone()) .chain(unread.cause.iter().cloned()) .collect(), - }; + ); machine_error( MachineAction::ListContainers, naming_machine(error, &silent.failure.machine_id), diff --git a/core/crates/ployz/src/deploy/exec_tests.rs b/core/crates/ployz/src/deploy/exec_tests.rs index 51181fc9f5..b05eb901c3 100644 --- a/core/crates/ployz/src/deploy/exec_tests.rs +++ b/core/crates/ployz/src/deploy/exec_tests.rs @@ -893,7 +893,7 @@ impl Scripted { } if holding && self.refused_held.contains(&container_id) { return Err(RpcError { - code: RpcErrorCode::from(crate::connect::FORBIDDEN), + code: RpcErrorCode::from(ployz_core::FORBIDDEN_CODE), message: "ployz.restart.device-approval.v2 needs a Device-signed restart approval" .into(), details: serde_json::Value::Null, diff --git a/core/crates/ployz/src/deploy/pipeline.rs b/core/crates/ployz/src/deploy/pipeline.rs index 7c85672a09..524f200592 100644 --- a/core/crates/ployz/src/deploy/pipeline.rs +++ b/core/crates/ployz/src/deploy/pipeline.rs @@ -9,7 +9,7 @@ use std::net::IpAddr; use ployz_core::{ DataLossConfirmation, DeploymentLogId, IngressHost, MachineId, MachineObservation, Namespace, - ObservedDataLoss, PortPublication, RpcError, RpcErrorCode, UnconfirmedDataLoss, + ObservedDataLoss, PortPublication, RpcError, UnconfirmedDataLoss, }; use thiserror::Error; use tokio_util::sync::CancellationToken; @@ -169,7 +169,7 @@ impl Client { .require(confirm_data_loss) .map_err(UnconfirmedDataLoss::into_rpc_error)?; if let Some(reason) = preview.prune_refusal { - return Err(invalid_argument(format!( + return Err(RpcError::invalid_argument(format!( "{reason}: {}", preview .warnings @@ -228,7 +228,7 @@ fn observed_destroy_loss( if volumes == super::VolumeFate::Destroy && let Some(reason) = preview.prune_refusal { - return Err(invalid_argument(format!( + return Err(RpcError::invalid_argument(format!( "{reason}: {}", preview .warnings @@ -243,15 +243,10 @@ fn observed_destroy_loss( fn require_namespace_present(preview: &DeployPreview) -> Result<(), RpcError> { if namespace_not_found(preview) { - return Err(RpcError { - code: RpcErrorCode::NotFound, - message: format!( - "Namespace '{}' was not found in this Cluster observation. No changes made.", - preview.namespace - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::not_found(format!( + "Namespace '{}' was not found in this Cluster observation. No changes made.", + preview.namespace + ))); } Ok(()) } @@ -283,20 +278,11 @@ impl From for RpcError { match error { DeployError::Connect(error) => error.into(), DeployError::Plan(error) => error.into_rpc_error(), - DeployError::Namespace(error) => invalid_argument(error.to_string()), + DeployError::Namespace(error) => RpcError::invalid_argument(error.to_string()), } } } -fn invalid_argument(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::InvalidArgument, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - } -} - #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct PushedImage { pub image: String, diff --git a/core/crates/ployz/src/deploy/planning.rs b/core/crates/ployz/src/deploy/planning.rs index 073e82ae64..fc33196756 100644 --- a/core/crates/ployz/src/deploy/planning.rs +++ b/core/crates/ployz/src/deploy/planning.rs @@ -4,7 +4,7 @@ use ployz_core::{ BlockedService, ContainerAction, ContainerId, CopyRotation, DataLoss, DependencyCondition, HookContainer, IngressHost, MachineId, MachineObservation, MembershipObservation, Namespace, ObservedDataLoss, PreservedVolume, PruneRefusal, QualifiedService, RequestedServiceSpec, - RowStanding, RpcError, RpcErrorCode, ServiceContainer, ServiceId, ServiceMode, ServiceName, + RowStanding, RpcError, ServiceContainer, ServiceId, ServiceMode, ServiceName, ServiceObservation, ServicePlacementEligibility, ServicePlacementIneligibleReason, UpdateOrder, VolumeToCreate, explicit_ingress_hosts, hostname_owners, machine_matches_placement, same_service_mode_kind, @@ -1292,30 +1292,23 @@ fn assemble_plan( /// Why a Service that depends on a blocked one is blocked too. fn dependency_blocked(dependency: &ServiceName) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!("depends on {dependency}, which was not attempted"), - details: serde_json::json!({ "reason": "dependency_blocked", "dependency": dependency }), - cause: Vec::new(), - } + RpcError::conflict(format!("depends on {dependency}, which was not attempted")).with_details( + serde_json::json!({ "reason": "dependency_blocked", "dependency": dependency }), + ) } /// Why a Service handing a hostname to a blocked one is blocked too: replacing its copies /// would take the hostname from the only copies that answer. fn handover_blocked(handover: &HostnameHandover) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: format!( - "hands hostname {} to {}, which was not attempted", - handover.hostname, handover.to - ), - details: serde_json::json!({ - "reason": "dependency_blocked", - "dependency": handover.to, - "hostname": handover.hostname, - }), - cause: Vec::new(), - } + RpcError::conflict(format!( + "hands hostname {} to {}, which was not attempted", + handover.hostname, handover.to + )) + .with_details(serde_json::json!({ + "reason": "dependency_blocked", + "dependency": handover.to, + "hostname": handover.hostname, + })) } fn reject_hostname_conflicts( diff --git a/core/crates/ployz/src/drain/retirement.rs b/core/crates/ployz/src/drain/retirement.rs index 4bccf0498a..03f80cd076 100644 --- a/core/crates/ployz/src/drain/retirement.rs +++ b/core/crates/ployz/src/drain/retirement.rs @@ -3,7 +3,7 @@ use ployz_core::{ AnonymousVolumes, EnvironmentValues, LiveServices, Machine, MachineId, MachineName, - ObservedGlobalSlotSpec, QualifiedService, RpcError, RpcErrorCode, ServicePlacementEligibility, + ObservedGlobalSlotSpec, QualifiedService, RpcError, ServicePlacementEligibility, }; use tokio_util::sync::CancellationToken; @@ -73,12 +73,9 @@ impl RetireClient for Client { ServicePlacementEligibility::Ineligible(_) => { retire_copies(self, machine_id, identity, names, cancellation).await } - ServicePlacementEligibility::Eligible => Err(RpcError { - code: RpcErrorCode::Conflict, - message: "the Server accepts it again".into(), - details: serde_json::Value::Null, - cause: Vec::new(), - }), + ServicePlacementEligibility::Eligible => { + Err(RpcError::conflict("the Server accepts it again")) + } ServicePlacementEligibility::Unknown(reason) => Err(unknown_eligibility(reason)), } } @@ -185,32 +182,22 @@ fn ended(step: Step, names: &Names) -> Result<(), RpcError> { match step { Step::Done => Ok(()), Step::Refused(refusal) => Err(refusal.into_error()), - Step::NotConfirmed(machines) => Err(RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "not unlisted after {} s; the proxies on {} did not confirm", - ACK_WAIT.as_secs(), - machines - .machines - .iter() - .map(|id| server_name(names, id)) - .collect::>() - .join(", ") - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }), + Step::NotConfirmed(machines) => Err(RpcError::unavailable(format!( + "not unlisted after {} s; the proxies on {} did not confirm", + ACK_WAIT.as_secs(), + machines + .machines + .iter() + .map(|id| server_name(names, id)) + .collect::>() + .join(", ") + ))), Step::Cancelled => Err(cancelled()), } } fn cancelled() -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: "the drain was cancelled while it retired".into(), - details: serde_json::Value::Null, - cause: Vec::new(), - } + RpcError::unavailable("the drain was cancelled while it retired") } /// Retire `globals` on `server` only: each is stopped and removed there when fresh evidence diff --git a/core/crates/ployz/src/enrollment.rs b/core/crates/ployz/src/enrollment.rs index b40383d658..fff57defc2 100644 --- a/core/crates/ployz/src/enrollment.rs +++ b/core/crates/ployz/src/enrollment.rs @@ -3,8 +3,7 @@ pub mod local; use crate::connect::{Client, ConnectError}; use ployz_core::{ - EnrollmentAssignment, EnrollmentSnapshot, ListMachinesRequest, Registered, RpcError, - RpcErrorCode, op, + EnrollmentAssignment, EnrollmentSnapshot, ListMachinesRequest, Registered, RpcError, op, }; /// Observe the Entry Machine's enrollment facts before taking an operator lock. @@ -16,12 +15,7 @@ pub async fn observe_enrollment(entry: &mut Client) -> Result(ListMachinesRequest {}, None) .await? .enrollment - .ok_or_else(|| RpcError { - code: RpcErrorCode::Unavailable, - message: "Entry Machine returned no enrollment snapshot".into(), - details: serde_json::Value::Null, - cause: Vec::new(), - }) + .ok_or_else(|| RpcError::unavailable("Entry Machine returned no enrollment snapshot")) } /// Publish a durably saved assignment before bootstrapping the joining Machine. @@ -38,12 +32,7 @@ pub async fn publish_enrollment( &snapshot, std::slice::from_ref(assignment), ) - .map_err(|error| RpcError { - code: RpcErrorCode::Conflict, - message: error.to_string(), - details: serde_json::Value::Null, - cause: Vec::new(), - })?; + .map_err(|error| RpcError::conflict(error.to_string()))?; let mut request = assignment.request.clone(); request.assigned_subnet = Some(assignment.machine.subnet); request.runtime.clone_from(&assignment.machine.runtime); diff --git a/core/crates/ployz/src/failure.rs b/core/crates/ployz/src/failure.rs index 9e26227209..9067a034a1 100644 --- a/core/crates/ployz/src/failure.rs +++ b/core/crates/ployz/src/failure.rs @@ -349,12 +349,9 @@ impl Failure { .expect("error details object") .extend(self.added.clone()); } - RpcError { - code, - message: self.to_string(), - details, - cause: self.causes(), - } + RpcError::new(code, self.to_string()) + .with_details(details) + .with_causes(self.causes()) } pub(crate) fn json(&self) -> Value { @@ -702,12 +699,7 @@ fn provision_code(error: &ProvisionError) -> RpcErrorCode { /// A failed image copy as the SDK reports it. pub(crate) fn push_rpc_error(error: PushError) -> RpcError { - RpcError { - code: push_code(&error), - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - } + RpcError::new(push_code(&error), error.to_string()) } fn push_code(error: &PushError) -> RpcErrorCode { diff --git a/core/crates/ployz/src/global_catch_up.rs b/core/crates/ployz/src/global_catch_up.rs index b370859b2d..f96baec4ad 100644 --- a/core/crates/ployz/src/global_catch_up.rs +++ b/core/crates/ployz/src/global_catch_up.rs @@ -221,15 +221,10 @@ impl CatchUpClient for Client { // creation key's name, so an Out copy must go before a new one is made. let existing = match existing { Some(existing) if matches!(existing.rotation(), CopyRotation::Held(_)) => { - return Err(RpcError { - code: ployz_core::RpcErrorCode::Conflict, - message: format!( - "copy {} is stopped on this Machine by a user; start or remove it there first", - existing.container_id.as_str() - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::conflict(format!( + "copy {} is stopped on this Machine by a user; start or remove it there first", + existing.container_id.as_str() + ))); } Some(existing) if *existing.rotation() == CopyRotation::Out => { step_error(self.retire_copy(&Copy::observed(&existing), cancel).await?)?; @@ -254,12 +249,7 @@ impl CatchUpClient for Client { .await .map_err(|error| crate::ui::rpc_error(ployz_core::RpcErrorCode::Unavailable, &error))?; if let Some(error) = endpoint_capacity_error(1, capacity.as_ref()) { - return Err(RpcError { - code: ployz_core::RpcErrorCode::Conflict, - message: error.to_string(), - details: serde_json::Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::conflict(error.to_string())); } let now = ensure_retried(self, &target, RunTarget::New(new)).await?; Ok(now.present().map(Copy::observed)) @@ -351,32 +341,22 @@ async fn takes_ingress(client: &mut Client, machine_id: &MachineId) -> Result Result<(), RpcError> { match step { Step::Done => Ok(()), Step::Refused(refusal) => Err(refusal.into_error()), - Step::NotConfirmed(machines) => Err(RpcError { - code: ployz_core::RpcErrorCode::Unavailable, - message: format!( - "{} did not confirm the copy left its routes", - machines - .machines - .iter() - .map(MachineId::to_string) - .collect::>() - .join(", ") - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }), + Step::NotConfirmed(machines) => Err(RpcError::unavailable(format!( + "{} did not confirm the copy left its routes", + machines + .machines + .iter() + .map(MachineId::to_string) + .collect::>() + .join(", ") + ))), Step::Cancelled => Err(crate::ui::rpc_error( ployz_core::RpcErrorCode::Internal, &Failure::cancelled(), diff --git a/core/crates/ployz/src/global_slot.rs b/core/crates/ployz/src/global_slot.rs index d84c420282..9d06b73986 100644 --- a/core/crates/ployz/src/global_slot.rs +++ b/core/crates/ployz/src/global_slot.rs @@ -2,7 +2,7 @@ use ployz_core::{ InspectRequest, MachineId, MachineTarget, Namespace, ResolvedServiceSpec, RpcError, - RpcErrorCode, ServicePlacementEligibility, ServicePlacementUnknownReason, op, + ServicePlacementEligibility, ServicePlacementUnknownReason, op, }; use crate::connect::Client; @@ -33,12 +33,7 @@ pub(crate) async fn slot_eligibility( machine.id == *machine_id && details.phase == ployz_core::LocalMachinePhase::Participating }) - .ok_or_else(|| RpcError { - code: RpcErrorCode::Conflict, - message: "the Server does not observe itself participating".into(), - details: serde_json::Value::Null, - cause: Vec::new(), - })?; + .ok_or_else(|| RpcError::conflict("the Server does not observe itself participating"))?; Ok(spec.placement_eligibility_in_namespace(namespace, &machine, details.storage.as_ref())) } @@ -49,10 +44,5 @@ pub(crate) fn unknown_eligibility(reason: ServicePlacementUnknownReason) -> RpcE "its storage support was not observed" } }; - RpcError { - code: RpcErrorCode::Conflict, - message: format!("eligibility on the Server is unknown: {why}"), - details: serde_json::Value::Null, - cause: Vec::new(), - } + RpcError::conflict(format!("eligibility on the Server is unknown: {why}")) } diff --git a/core/crates/ployz/src/handlers/cloud_read.rs b/core/crates/ployz/src/handlers/cloud_read.rs index 3b9ffc8c37..f0fee12c74 100644 --- a/core/crates/ployz/src/handlers/cloud_read.rs +++ b/core/crates/ployz/src/handlers/cloud_read.rs @@ -51,7 +51,7 @@ fn held_elsewhere(credential: &Credential, scoped: &Scoped) -> Result<(), Error> }; match refusable::(credential, asked)? { Err(StoreCallError::Refused(refused)) - if refused.code.as_str() == "forbidden" + if refused.code.as_str() == ployz_core::FORBIDDEN_CODE && refused .details .get("environments") diff --git a/core/crates/ployz/src/handlers/deploy/progress.rs b/core/crates/ployz/src/handlers/deploy/progress.rs index 7721150ce8..670b83dd3d 100644 --- a/core/crates/ployz/src/handlers/deploy/progress.rs +++ b/core/crates/ployz/src/handlers/deploy/progress.rs @@ -539,13 +539,7 @@ fn superseded( command(&[&["deployment", "ls"][..], &scope].concat()), ), }; - Failure::from(RpcError { - code: RpcErrorCode::Internal, - message, - cause: Vec::new(), - details: serde_json::Value::Null, - }) - .hint(Hint::Inspect(inspect)) + Failure::from(RpcError::internal(message)).hint(Hint::Inspect(inspect)) } /// The Deployment among `candidates` that took `view`'s Environment over: the first @@ -774,16 +768,17 @@ fn failure(view: &DeploymentView, named: &[&str], command: impl Fn(&[&str]) -> S // failure the inspect hint above reads. bad_image |= pulled && !build_failed && built.is_none(); } - let mut failure = Failure::from(RpcError { - code: if lost_to_newer { - RpcErrorCode::Conflict - } else { - RpcErrorCode::Internal - }, - message: crate::ui::sentence(reason), - cause, - details: serde_json::Value::Null, - }) + let mut failure = Failure::from( + RpcError::new( + if lost_to_newer { + RpcErrorCode::Conflict + } else { + RpcErrorCode::Internal + }, + crate::ui::sentence(reason), + ) + .with_causes(cause), + ) .with_diagnostic(diagnostic); for hint in hints { failure = failure.hint(hint); diff --git a/core/crates/ployz/src/handlers/mcp.rs b/core/crates/ployz/src/handlers/mcp.rs index d64780fab4..40f8322451 100644 --- a/core/crates/ployz/src/handlers/mcp.rs +++ b/core/crates/ployz/src/handlers/mcp.rs @@ -323,15 +323,11 @@ fn written(answer: Result) -> Result { /// A write ployz mcp stopped waiting for after sending it. fn cut_off(tool: &str, deadline: Duration) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "Stopped waiting for `{tool}` after {}; Ployz Cloud may still take what it sent", - span(deadline) - ), - details: json!({ "writes": "unknown" }), - cause: Vec::new(), - } + RpcError::unavailable(format!( + "Stopped waiting for `{tool}` after {}; Ployz Cloud may still take what it sent", + span(deadline) + )) + .with_details(json!({ "writes": "unknown" })) } /// Whether Cloud may have acted on a request that failed this way. @@ -364,12 +360,10 @@ fn may_have_landed(error: &LoginError) -> bool { /// The registry refuses Cloud tools on a local Store before asking; this guards the rest. fn not_here() -> RpcError { - RpcError { - code: RpcErrorCode::from("not_here"), - message: "This needs Ployz Cloud; a local Config Store cannot answer it".to_owned(), - details: Value::Null, - cause: Vec::new(), - } + RpcError::new( + RpcErrorCode::from("not_here"), + "This needs Ployz Cloud; a local Config Store cannot answer it", + ) } /// The Project and Environment a call acts on, read when the call starts. @@ -628,7 +622,7 @@ impl Server { self.forget(tool, context).await; } match error.details.get("approval_id").and_then(Value::as_str) { - Some(id) if error.code.as_str() == approval::APPROVAL_REQUIRED => { + Some(id) if error.code.as_str() == ployz_core::APPROVAL_REQUIRED_CODE => { let url = waiting_on_you(&credential).await; let text = format!( "{message} Nothing ran; the approval stays pending. Ask a human to \ diff --git a/core/crates/ployz/src/handlers/server.rs b/core/crates/ployz/src/handlers/server.rs index afd26aa654..fab618f5d5 100644 --- a/core/crates/ployz/src/handlers/server.rs +++ b/core/crates/ployz/src/handlers/server.rs @@ -421,7 +421,9 @@ pub(super) async fn reach_or_silent( if let Ok(credential) = cloud_account::from_env(&store).await { let access = match cloud_account::server_access(&credential).await { Ok(access) => access, - Err(StoreCallError::Refused(error)) if error.code.as_str() == "forbidden" => { + Err(StoreCallError::Refused(error)) + if error.code.as_str() == ployz_core::FORBIDDEN_CODE => + { return Ok(Some(Reach::Ask(credential))); } Err(error) => return Err(Error::from(error).into()), diff --git a/core/crates/ployz/src/handlers/service/mod.rs b/core/crates/ployz/src/handlers/service/mod.rs index a236595742..426cc2eccc 100644 --- a/core/crates/ployz/src/handlers/service/mod.rs +++ b/core/crates/ployz/src/handlers/service/mod.rs @@ -8,8 +8,8 @@ use crate::cli::{base, positional, value}; use ployz_core::{ ContainerAction, ContainerId, ContainerObservation, ContainerRef, ContainerRuntimeObservation, CopyRotation, HealthFailure, HealthObservation, LiveServices, MachineFailure, MachineId, - MachineName, MachineObservation, OperationId, QualifiedService, RpcError, RpcErrorCode, - Sealing, ServiceContainer, ServiceObservation, ServiceSelector, + MachineName, MachineObservation, OperationId, QualifiedService, RpcError, Sealing, + ServiceContainer, ServiceObservation, ServiceSelector, authority::{RestartApproval, Signed, Signer, SignerKey, Timestamp, WorkflowKind}, select_service, }; @@ -719,12 +719,10 @@ async fn apply_service_action( } fn unhealthy(failure: &HealthFailure) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: format!("The Container's healthcheck did not pass: {failure}."), - details: serde_json::to_value(failure).unwrap_or_default(), - cause: Vec::new(), - } + RpcError::unavailable(format!( + "The Container's healthcheck did not pass: {failure}." + )) + .with_details(serde_json::to_value(failure).unwrap_or_default()) } fn change_selectors( diff --git a/core/crates/ployz/src/handlers/teardown.rs b/core/crates/ployz/src/handlers/teardown.rs index 4b18e8c514..a332cbb2e0 100644 --- a/core/crates/ployz/src/handlers/teardown.rs +++ b/core/crates/ployz/src/handlers/teardown.rs @@ -372,16 +372,14 @@ fn bound( if let Some((shown, listed)) = &prompted { let since = shown.changes(&listed().map_err(unbound)?); if !since.is_empty() { - return Err(unbound(StoreCallError::Refused(ployz_core::RpcError { - code: RpcErrorCode::ConfirmationRequired, - message: format!( + return Err(unbound(StoreCallError::Refused( + ployz_core::RpcError::confirmation_required(format!( "What this removes changed since you confirmed: {}. Nothing was \ removed. Run it again to go ahead", since.join(", ") - ), - details: json!({}), - cause: Vec::new(), - }))); + )) + .with_details(json!({})), + ))); } } if request.version.is_some() { diff --git a/core/crates/ployz/src/ingress.rs b/core/crates/ployz/src/ingress.rs index 3298f70e74..99506c39d5 100644 --- a/core/crates/ployz/src/ingress.rs +++ b/core/crates/ployz/src/ingress.rs @@ -367,12 +367,7 @@ pub(crate) fn unsupported(server: &MachineName, capability: &str) -> RpcError { }); let refusal = ployz_core::CapabilityRefusal::new(&RpcErrorCode::Unsupported, details.clone()) .expect("the details name a target and a capability"); - RpcError { - code: RpcErrorCode::Unsupported, - message: refusal.to_string(), - details, - cause: Vec::new(), - } + RpcError::unsupported(refusal.to_string()).with_details(details) } /// One Server's Ingress Proxy after [`follow_roles`] set it. @@ -529,16 +524,11 @@ pub(crate) async fn use_doors( } Some(failure) => failures.push(MachineFailure { machine_id: machine.id, - error: RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "The Ingress Proxy on {} isn't running: {}", - machine.name, - failure.cause() - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error: RpcError::unavailable(format!( + "The Ingress Proxy on {} isn't running: {}", + machine.name, + failure.cause() + )), }), }, Answer::Refused(error) => failures.push(MachineFailure { @@ -547,15 +537,10 @@ pub(crate) async fn use_doors( }), Answer::Cancelled => failures.push(MachineFailure { machine_id: machine.id, - error: RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "{} was still setting its Doors when the command was cancelled", - machine.name - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error: RpcError::unavailable(format!( + "{} was still setting its Doors when the command was cancelled", + machine.name + )), }), } } diff --git a/core/crates/ployz/src/operator/history.rs b/core/crates/ployz/src/operator/history.rs index b217471bbf..4356690bc1 100644 --- a/core/crates/ployz/src/operator/history.rs +++ b/core/crates/ployz/src/operator/history.rs @@ -286,9 +286,9 @@ async fn read_page( loop { let message = tokio::time::timeout(ROW_DEADLINE, stream.message()) .await - .map_err(|_| unavailable("the Server's Log Store stopped answering"))? + .map_err(|_| RpcError::unavailable("the Server's Log Store stopped answering"))? .map_err(|status| RpcError::from(ConnectError::from(status)))? - .ok_or_else(|| unavailable("the Server's Log Store ended a page early"))?; + .ok_or_else(|| RpcError::unavailable("the Server's Log Store ended a page early"))?; let row = HistoryRow::decode(&message) .map_err(|error| crate::ui::rpc_error(RpcErrorCode::Internal, &error))?; let (container_id, event) = match row { @@ -300,7 +300,7 @@ async fn read_page( } HistoryRow::Heartbeat => continue, HistoryRow::End { next } => return Ok(next), - HistoryRow::Error(message) => return Err(unavailable(&message)), + HistoryRow::Error(message) => return Err(RpcError::unavailable(&message)), HistoryRow::Line { container_id, ts, @@ -351,12 +351,3 @@ fn undescribed(container_id: ContainerId) -> HistoryContainer { kind: HistoryContainerKind::Service, } } - -fn unavailable(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: message.to_owned(), - details: serde_json::Value::Null, - cause: Vec::new(), - } -} diff --git a/core/crates/ployz/src/private_access.rs b/core/crates/ployz/src/private_access.rs index 9a373824a3..c90d09a50a 100644 --- a/core/crates/ployz/src/private_access.rs +++ b/core/crates/ployz/src/private_access.rs @@ -234,15 +234,10 @@ pub(crate) async fn turn_on( }), Answer::Cancelled => changed.failures.push(MachineFailure { machine_id: machine.id, - error: RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "{} was still starting its router when the command was cancelled", - machine.name - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error: RpcError::unavailable(format!( + "{} was still starting its router when the command was cancelled", + machine.name + )), }), } } @@ -284,15 +279,10 @@ pub(crate) async fn turn_off( }), Answer::Cancelled => changed.failures.push(MachineFailure { machine_id: machine.id, - error: RpcError { - code: RpcErrorCode::Unavailable, - message: format!( - "{} was still removing its router when the command was cancelled", - machine.name - ), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error: RpcError::unavailable(format!( + "{} was still removing its router when the command was cancelled", + machine.name + )), }), } } @@ -310,16 +300,13 @@ fn not_serving(server: &MachineName, problem: PrivateAccessProblem, detail: Stri format!("The Tailscale router on {server} isn't running.") } }; - RpcError { - code: RpcErrorCode::Unavailable, - message, - details: serde_json::json!({ "problem": problem }), - cause: if detail.is_empty() { + RpcError::unavailable(message) + .with_details(serde_json::json!({ "problem": problem })) + .with_causes(if detail.is_empty() { Vec::new() } else { vec![detail] - }, - } + }) } /// Whether `error` is a Machine's answer that its router isn't serving, not a Machine diff --git a/core/crates/ployz/src/published/mod.rs b/core/crates/ployz/src/published/mod.rs index 113292903c..51ae2e06ed 100644 --- a/core/crates/ployz/src/published/mod.rs +++ b/core/crates/ployz/src/published/mod.rs @@ -148,7 +148,7 @@ impl PublishError { } Self::Gone { .. } => RpcErrorCode::NotFound, Self::NeedsTerminal { .. } => RpcErrorCode::ConfirmationRequired, - Self::AgentRefused { .. } => RpcErrorCode::from("forbidden"), + Self::AgentRefused { .. } => RpcErrorCode::from(ployz_core::FORBIDDEN_CODE), Self::OtherCloud { .. } => RpcErrorCode::Conflict, Self::Slow { .. } => RpcErrorCode::Unavailable, } diff --git a/core/crates/ployz/src/sdk.rs b/core/crates/ployz/src/sdk.rs index e0400cbbbd..b2f4e878ed 100644 --- a/core/crates/ployz/src/sdk.rs +++ b/core/crates/ployz/src/sdk.rs @@ -201,10 +201,10 @@ fn volume_switch_path(body: &RpcRequestBody) -> Result<&'static str, RpcError> { Body::InspectVolumeCopy(_) | Body::SeedVolume(_) | Body::SetVolumeCopy(_) - | Body::SetVolumeSize(_) => body - .unary_path() - .ok_or_else(|| invalid_argument(format!("{} is not a unary RPC", body.command()))), - _ => Err(invalid_argument(format!( + | Body::SetVolumeSize(_) => body.unary_path().ok_or_else(|| { + RpcError::invalid_argument(format!("{} is not a unary RPC", body.command())) + }), + _ => Err(RpcError::invalid_argument(format!( "{} is not a Volume switch command", body.command() ))), @@ -223,19 +223,15 @@ fn volume_switch_payload(command: &str, body: RpcResponseBody) -> Result serde_json::to_value(reply), RpcResponseBody::Error(error) => return Err(error), other => { - return Err(RpcError { - code: RpcErrorCode::Internal, - message: format!("Machine answered {command} with {}", other.kind().as_str()), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::internal(format!( + "Machine answered {command} with {}", + other.kind().as_str() + ))); } }; - payload.map_err(|error| RpcError { - code: RpcErrorCode::Internal, - message: "Volume switch reply could not be encoded".into(), - details: Value::Null, - cause: vec![error.to_string()], + payload.map_err(|error| { + RpcError::internal("Volume switch reply could not be encoded") + .with_causes(vec![error.to_string()]) }) } @@ -362,7 +358,7 @@ pub async fn connect_expecting( expect: Option, ) -> Result { if connections.is_empty() { - return Err(invalid_argument("connections must not be empty".into()).into()); + return Err(RpcError::invalid_argument("connections must not be empty").into()); } Ok(dial(connections, connector, expect).await?) } @@ -486,11 +482,7 @@ impl Session { let mut client = self.client()?; tokio::select! { biased; - () = self.inner.cancel.cancelled() => Err(RpcError { - code: RpcErrorCode::Unavailable, - message: "session closed; in-flight Register outcome may be uncertain".into(), - details: Value::Null, cause: Vec::new(), - }), + () = self.inner.cancel.cancelled() => Err(RpcError::unavailable("session closed; in-flight Register outcome may be uncertain")), result = crate::enrollment::publish_enrollment(&mut client, assignment) => result, } } @@ -530,15 +522,10 @@ impl Session { let description = self.about().await?; if !description.supports(ployz_core::SET_PUSH_TARGET_CAPABILITY) { return Err(name_refusal( - RpcError { - code: RpcErrorCode::Unsupported, - message: format!( - "{} is not advertised", - ployz_core::SET_PUSH_TARGET_CAPABILITY - ), - details: Value::Null, - cause: Vec::new(), - }, + RpcError::unsupported(format!( + "{} is not advertised", + ployz_core::SET_PUSH_TARGET_CAPABILITY + )), op::SetPushTarget::PATH, &MachineTarget::from(&description.machine_id), )); @@ -561,15 +548,10 @@ impl Session { let description = self.about().await?; if !description.supports(ployz_core::SET_CLUSTER_NAME_CAPABILITY) { return Err(name_refusal( - RpcError { - code: RpcErrorCode::Unsupported, - message: format!( - "{} is not advertised", - ployz_core::SET_CLUSTER_NAME_CAPABILITY - ), - details: Value::Null, - cause: Vec::new(), - }, + RpcError::unsupported(format!( + "{} is not advertised", + ployz_core::SET_CLUSTER_NAME_CAPABILITY + )), op::SetClusterName::PATH, &MachineTarget::from(&description.machine_id), )); @@ -593,13 +575,8 @@ impl Session { ) .await?; response.capability.ok_or_else(|| { - RpcError { - code: RpcErrorCode::Internal, - message: "Machine set a Management Client without a Management Capability".into(), - details: Value::Null, - cause: Vec::new(), - } - .into() + RpcError::internal("Machine set a Management Client without a Management Capability") + .into() }) } @@ -621,8 +598,8 @@ impl Session { &self, machine: &str, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let client = self.client()?; self.until_closed(client.invoke::( ployz_core::InspectRequest::default(), @@ -707,12 +684,7 @@ impl Session { let entry = MachineTarget::from(&description.machine_id); if !description.supports(RUNTIME_WATCH_CAPABILITY) { return Err(name_refusal( - RpcError { - code: RpcErrorCode::Unsupported, - message: format!("{RUNTIME_WATCH_CAPABILITY} is not advertised"), - details: Value::Null, - cause: Vec::new(), - }, + RpcError::unsupported(format!("{RUNTIME_WATCH_CAPABILITY} is not advertised")), op::RuntimeWatch::PATH, &entry, )); @@ -1003,10 +975,10 @@ impl Session { /// `unanswered`: the Machine may have taken the request. One whose entry Machine /// could not dial `machine` is not, since `machine` never got it. pub async fn volume_switch(&self, machine: &str, request: Value) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; - let body: RpcRequestBody = - serde_json::from_value(request).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; + let body: RpcRequestBody = serde_json::from_value(request) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let path = volume_switch_path(&body)?; let request = ployz_core::RpcRequest::from(body); let client = self.client()?; @@ -1043,8 +1015,8 @@ impl Session { &self, machine: &str, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(client.data_loss_if_machine_removed(&target)) .await @@ -1069,8 +1041,8 @@ impl Session { machine: &str, confirm_data_loss: &DataLossConfirmation, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(client.remove_machine( &target, @@ -1100,8 +1072,8 @@ impl Session { machine: &str, scope: &crate::drain::DrainScope, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; client .drain(&target, scope, &self.inner.cancel, &mut |_| {}) @@ -1143,8 +1115,8 @@ impl Session { &self, machine: &str, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; let cancel = &self.inner.cancel; self.until_closed(async { @@ -1160,7 +1132,9 @@ impl Session { crate::cluster::RoleWaitError::Connect(error) => RpcError::from(error), error @ (crate::cluster::RoleWaitError::NotObserved(_) | crate::cluster::RoleWaitError::ChangeNotObserved - | crate::cluster::RoleWaitError::Cancelled) => unavailable(error.to_string()), + | crate::cluster::RoleWaitError::Cancelled) => { + RpcError::unavailable(error.to_string()) + } })?; crate::ingress::fill_slots(&mut client, cancel) .await @@ -1188,13 +1162,15 @@ impl Session { .map(|door| { serde_json::from_value::(Value::String(door.clone())) .map_err(|_| { - invalid_argument(format!("{door} is not a Door: public or tunnel")) + RpcError::invalid_argument(format!( + "{door} is not a Door: public or tunnel" + )) }) }) .collect::, _>>()?; if doors.is_empty() { - return Err(invalid_argument( - "name at least one Door: public or tunnel".into(), + return Err(RpcError::invalid_argument( + "name at least one Door: public or tunnel", )); } let doors = ployz_core::IngressDoors(doors); @@ -1229,8 +1205,8 @@ impl Session { ) -> Result { let auth_key = auth_key.map(str::trim).filter(|key| !key.is_empty()); if enabled && auth_key.is_none() { - return Err(invalid_argument( - "Private Access needs a Tailscale auth key.".into(), + return Err(RpcError::invalid_argument( + "Private Access needs a Tailscale auth key.", )); } let mut client = self.client()?; @@ -1262,8 +1238,8 @@ impl Session { /// Machine Target or not visible, this entry hasn't observed the role within 30 s, or /// the Machine does not acknowledge a listing, stop or removal. pub async fn stop_ingress(&self, machine: &str) -> Result<(), RpcError> { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; let cancel = &self.inner.cancel; self.until_closed(async { @@ -1279,7 +1255,9 @@ impl Session { crate::cluster::RoleWaitError::Connect(error) => RpcError::from(error), error @ (crate::cluster::RoleWaitError::NotObserved(_) | crate::cluster::RoleWaitError::ChangeNotObserved - | crate::cluster::RoleWaitError::Cancelled) => unavailable(error.to_string()), + | crate::cluster::RoleWaitError::Cancelled) => { + RpcError::unavailable(error.to_string()) + } })?; crate::drain::retirement::retire_ingress_proxy(&client, &id, cancel).await }) @@ -1295,8 +1273,8 @@ impl Session { /// Machine Target or not visible, it is the last Machine and a Management Client /// holds a key or its holders can't be read, or shared-row removal fails. pub async fn remove_machine_membership(&self, machine: &str) -> Result<(), RpcError> { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(client.remove_machine_membership(&target)) .await @@ -1314,10 +1292,10 @@ impl Session { machine: &str, container: &str, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let container_id = ployz_core::ContainerId::parse(container) - .map_err(|error| invalid_argument(error.to_string()))?; + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let client = self.client()?; let details = self .until_closed(client.invoke::( @@ -1326,12 +1304,8 @@ impl Session { Some(crate::connect::TARGET_RPC_TIMEOUT), )) .await?; - serde_json::to_value(details).map_err(|error| RpcError { - code: RpcErrorCode::Internal, - message: format!("encoding Container {container}: {error}"), - details: Value::Null, - cause: Vec::new(), - }) + serde_json::to_value(details) + .map_err(|error| RpcError::internal(format!("encoding Container {container}: {error}"))) } /// Copy the image Container `container` runs on `source` to `dest`, by its local image @@ -1350,11 +1324,12 @@ impl Session { dest: &str, ) -> Result<(), RpcError> { let parse = |machine: &str| { - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string())) + MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string())) }; let (source, dest) = (parse(source)?, parse(dest)?); let container_id = ployz_core::ContainerId::parse(container) - .map_err(|error| invalid_argument(error.to_string()))?; + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(async { let machines = client.machines().await.map_err(RpcError::from)?; @@ -1371,14 +1346,11 @@ impl Session { Some(crate::connect::TARGET_RPC_TIMEOUT), ) .await?; - let image_id = details.image_id.ok_or_else(|| RpcError { - code: RpcErrorCode::Unsupported, - message: format!( + let image_id = details.image_id.ok_or_else(|| { + RpcError::unsupported(format!( "{} does not report the image ID its Containers run", source.name - ), - details: Value::Null, - cause: Vec::new(), + )) })?; let image = &details.container.resolved_spec.container.image; crate::image::copy_running_image(&client, &source, &dest, image, &image_id) @@ -1402,8 +1374,8 @@ impl Session { machine: &str, update: ployz_core::MachineUpdate, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let client = self.client()?; self.until_closed(client.invoke::( ployz_core::UpdateMachineRequest { update }, @@ -1439,8 +1411,8 @@ impl Session { machine: &str, request: T::Request, ) -> Result { - let target = - MachineTarget::parse(machine).map_err(|error| invalid_argument(error.to_string()))?; + let target = MachineTarget::parse(machine) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(client.read::(request, &target)).await } @@ -1459,8 +1431,8 @@ impl Session { namespace: &str, volumes: VolumeFate, ) -> Result { - let namespace = - Namespace::parse(namespace).map_err(|error| invalid_argument(error.to_string()))?; + let namespace = Namespace::parse(namespace) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(client.data_loss_if_namespace_destroyed(&namespace, volumes)) .await @@ -1486,8 +1458,8 @@ impl Session { confirm_data_loss: &DataLossConfirmation, volumes: VolumeFate, ) -> Result, RpcError> { - let namespace = - Namespace::parse(namespace).map_err(|error| invalid_argument(error.to_string()))?; + let namespace = Namespace::parse(namespace) + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; let mut client = self.client()?; self.until_closed(client.destroy_namespace( &namespace, @@ -1605,12 +1577,9 @@ impl Watch { if self.cancel.is_cancelled() { Ok(None) } else { - Err(RpcError { - code: RpcErrorCode::Unavailable, - message: "Watch stream ended; reconnect to resume".into(), - details: Value::Null, - cause: Vec::new(), - }) + Err(RpcError::unavailable( + "Watch stream ended; reconnect to resume", + )) } } Some(Ok(Some(payload))) => match decode_runtime_watch_frame(&payload) { @@ -1667,30 +1636,7 @@ impl Watch { } fn closed() -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: "client is closed".into(), - details: Value::Null, - cause: Vec::new(), - } -} - -fn invalid_argument(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::InvalidArgument, - message, - details: Value::Null, - cause: Vec::new(), - } -} - -fn unavailable(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message, - details: Value::Null, - cause: Vec::new(), - } + RpcError::unavailable("client is closed") } /// The ID of the one visible Machine `target` names. @@ -1708,7 +1654,7 @@ async fn visible_machine_id( async fn capture(input: PreparationInput) -> Result { tokio::task::spawn_blocking(move || preparation::capture(input)) .await - .map_err(|_| invalid_argument("source capture task failed".into()))? + .map_err(|_| RpcError::invalid_argument("source capture task failed"))? } pub(crate) fn preparation_error( @@ -1735,25 +1681,24 @@ pub(crate) fn preparation_error( .collect::>(); let message = "No eligible Build Machine was selected; no build was started.".to_owned(); - RpcError { - code: RpcErrorCode::Unavailable, - details: serde_json::json!({"preparation":{"kind":"failed", "stage":"Selection", + RpcError::unavailable(message.clone()) + .with_details( + serde_json::json!({"preparation":{"kind":"failed", "stage":"Selection", "message":message, "rejections":rejections}}), - message, - cause: if reasons.is_empty() { + ) + .with_causes(if reasons.is_empty() { Vec::new() } else { vec![reasons.join("; ")] - }, - } + }) } - PreparationError::Connect(_) => RpcError { - code: RpcErrorCode::Unavailable, - message: "Could not read Machine observations during preparation.".into(), - details: serde_json::json!({"preparation":{"kind":"failed", "stage":"Observation", + PreparationError::Connect(_) => { + RpcError::unavailable("Could not read Machine observations during preparation.") + .with_details( + serde_json::json!({"preparation":{"kind":"failed", "stage":"Observation", "message":"Could not read Machine observations during preparation."}}), - cause: Vec::new(), - }, + ) + } PreparationError::Build(crate::build::Error::RemoteBuild { outcome }) => { let cancelled = cancellation_requested && matches!(*outcome, crate::build::RemoteBuildFailure::Failed { .. }); @@ -1765,20 +1710,11 @@ pub(crate) fn preparation_error( .expect("remote Build failures serialize a kind") = serde_json::json!("cancelled"); } - RpcError { - code: RpcErrorCode::Internal, - message, - details, - cause: Vec::new(), - } + RpcError::internal(message).with_details(details) } PreparationError::UploadNeeded(services) => preparation::upload_needed(&services), - PreparationError::Cancelled => RpcError { - code: RpcErrorCode::Unavailable, - message, - details: serde_json::json!({"preparation":{"kind":"cancelled"}}), - cause: Vec::new(), - }, + PreparationError::Cancelled => RpcError::unavailable(message) + .with_details(serde_json::json!({"preparation":{"kind":"cancelled"}})), PreparationError::Build(_) | PreparationError::Plan(_) | PreparationError::Delivery(_) => { let mut details = serde_json::Map::new(); details.insert( @@ -1788,12 +1724,7 @@ pub(crate) fn preparation_error( if same_again { details.insert("same_again".into(), Value::Bool(true)); } - RpcError { - code: RpcErrorCode::Internal, - details: Value::Object(details), - message, - cause: Vec::new(), - } + RpcError::internal(message).with_details(Value::Object(details)) } } } diff --git a/core/crates/ployz/src/sdk/agent.rs b/core/crates/ployz/src/sdk/agent.rs index 010c9cd049..92cbe74162 100644 --- a/core/crates/ployz/src/sdk/agent.rs +++ b/core/crates/ployz/src/sdk/agent.rs @@ -9,8 +9,7 @@ use ployz_core::metrics::{MarkerKind, MetricMarker, MetricRange}; use ployz_core::{ ContainerId, ContainerKind, ContainerObservation, ContainerRuntimeObservation, EnvironmentValues, HealthObservation, HealthVerdict, LogHistoryRequest, MachineId, - MachineObservation, MachineStorageObservation, Namespace, RpcError, RpcErrorCode, ServiceName, - StopOptions, + MachineObservation, MachineStorageObservation, Namespace, RpcError, ServiceName, StopOptions, }; use ployz_store::DeploymentId; use ployz_store::tools::cloud::{ @@ -70,14 +69,14 @@ pub async fn ask( CloudQuery::Logs(query) => encode(&logs(session, query, sealed).await?), CloudQuery::ServerLogs(query) => encode(&server_logs(session, query).await?), CloudQuery::Metrics(query) => encode(&metrics(session, query, readable).await?), - CloudQuery::Copies { .. } => Err(unsupported( + CloudQuery::Copies { .. } => Err(RpcError::unsupported( "Copies join the Config Store; Ployz Cloud answers them", )), CloudQuery::Operations(_) | CloudQuery::GithubTree(_) | CloudQuery::GithubFile(_) | CloudQuery::GithubConnection - | CloudQuery::GithubBranches { .. } => Err(unsupported( + | CloudQuery::GithubBranches { .. } => Err(RpcError::unsupported( "Operations and GitHub are Ployz Cloud's records; the Servers do not hold them", )), } @@ -753,21 +752,8 @@ fn now() -> Timestamp { } fn encode(answer: &impl serde::Serialize) -> Result { - serde_json::to_value(answer).map_err(|error| RpcError { - code: RpcErrorCode::Internal, - message: format!("encoding a Cloud answer: {error}"), - details: Value::Null, - cause: Vec::new(), - }) -} - -fn unsupported(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Unsupported, - message: message.to_owned(), - details: Value::Null, - cause: Vec::new(), - } + serde_json::to_value(answer) + .map_err(|error| RpcError::internal(format!("encoding a Cloud answer: {error}"))) } #[cfg(test)] diff --git a/core/crates/ployz/src/sdk/build.rs b/core/crates/ployz/src/sdk/build.rs index 48940e56ac..2aa10a3f1c 100644 --- a/core/crates/ployz/src/sdk/build.rs +++ b/core/crates/ployz/src/sdk/build.rs @@ -38,10 +38,9 @@ pub(super) async fn run( .chain(captured.reused.keys()) .collect(); if captured.build.targets().count() > 1 || services.len() != 1 { - return Err(super::invalid_argument( + return Err(RpcError::invalid_argument( "build input must hold exactly one Git Service with its source commit, \ - or one uploaded Service" - .into(), + or one uploaded Service", )); } let admit_by = start_within.map(|limit| tokio::time::Instant::now() + limit); @@ -68,7 +67,7 @@ pub(super) async fn run( }), receipt, }) - .ok_or_else(|| super::invalid_argument("Build produced no receipt".into())), + .ok_or_else(|| RpcError::invalid_argument("Build produced no receipt")), Err(PreparationError::Build(crate::build::Error::Withdrawn)) => Ok(BuildOutcome::Queued), Err(error) => Err(build_error(error, token.is_cancelled())), } @@ -119,7 +118,7 @@ pub(super) async fn outside( return Ok(reused); } let platforms = crate::build::placement_platforms(&intent, &machines) - .map_err(|error| super::invalid_argument(error.to_string()))?; + .map_err(|error| RpcError::invalid_argument(error.to_string()))?; Ok(OutsideBuild::Build { platforms: platforms.into_iter().collect(), }) @@ -134,8 +133,8 @@ async fn reuse( token: &CancellationToken, ) -> Result, RpcError> { let [spec] = intent.target.as_slice() else { - return Err(super::invalid_argument( - "the deployment must hold exactly one Service".into(), + return Err(RpcError::invalid_argument( + "the deployment must hold exactly one Service", )); }; // Fingerprints are keyed by Service, as `capture` computes them. diff --git a/core/crates/ployz/src/sdk/deploy.rs b/core/crates/ployz/src/sdk/deploy.rs index fc9e396837..dcb0004957 100644 --- a/core/crates/ployz/src/sdk/deploy.rs +++ b/core/crates/ployz/src/sdk/deploy.rs @@ -1,8 +1,7 @@ //! Prepared deployment execution and its owned progress stream. -use super::{PreparedDeploy, RunningDeploy, Session, closed, invalid_argument}; +use super::{PreparedDeploy, RunningDeploy, Session, closed}; use crate::deploy::DeployPreview; -use ployz_core::{DeployEvent, DeployOutcome, ExecutionError, RpcError, RpcErrorCode}; -use serde_json::Value; +use ployz_core::{DeployEvent, DeployOutcome, ExecutionError, RpcError}; use std::{ops::Deref, sync::atomic::Ordering}; use tokio::sync::{Mutex, mpsc}; @@ -23,7 +22,7 @@ impl std::str::FromStr for ImageCleanup { match value { "auto" => Ok(Self::Auto), "manual" => Ok(Self::Manual), - other => Err(invalid_argument(format!( + other => Err(RpcError::invalid_argument(format!( "imageCleanup must be \"auto\" or \"manual\", not {other:?}" ))), } @@ -131,8 +130,8 @@ impl PreparedDeploy { } let mut retained = self.retained.lock().expect("retained build lock"); if self.confirmed.swap(true, Ordering::SeqCst) { - return Err(invalid_argument( - "this Deploy Preview already confirmed".into(), + return Err(RpcError::invalid_argument( + "this Deploy Preview already confirmed", )); } let cancel = session.inner.cancel.child_token(); @@ -155,11 +154,7 @@ impl PreparedDeploy { let events = tx.clone(); let outcome = tokio::select! { biased; - () = session_cancel.cancelled() => return Err(RpcError { - code: RpcErrorCode::Unavailable, - message: "session closed; in-flight Deploy outcome may be uncertain".into(), - details: Value::Null, cause: Vec::new(), - }), + () = session_cancel.cancelled() => return Err(RpcError::unavailable("session closed; in-flight Deploy outcome may be uncertain")), outcome = client.confirm(&preview, &token, Some(tx)) => outcome, }; if !token.is_cancelled() { @@ -226,13 +221,10 @@ impl RunningDeploy { .lock() .await .take() - .ok_or_else(|| invalid_argument("Deploy outcome already awaited".into()))?; - let outcome = handle.await.map_err(|_| RpcError { - code: RpcErrorCode::Internal, - message: "Deploy task failed".into(), - details: Value::Null, - cause: Vec::new(), - })?; + .ok_or_else(|| RpcError::invalid_argument("Deploy outcome already awaited"))?; + let outcome = handle + .await + .map_err(|_| RpcError::internal("Deploy task failed"))?; while self.next().await.is_some() {} outcome } @@ -241,6 +233,7 @@ impl RunningDeploy { #[cfg(test)] mod tests { use super::*; + use ployz_core::RpcErrorCode; #[tokio::test] async fn a_second_finished_is_a_typed_error() { diff --git a/core/crates/ployz/src/sdk/github_build.rs b/core/crates/ployz/src/sdk/github_build.rs index cd8059099d..1885d010cc 100644 --- a/core/crates/ployz/src/sdk/github_build.rs +++ b/core/crates/ployz/src/sdk/github_build.rs @@ -26,7 +26,7 @@ use serde_json::Value; use super::build::OutsideBuild; use super::preparation::{BuildReceipt, OutsideBuildInput}; use super::store_call; -use super::store_runner::{internal, log_line, only}; +use super::store_runner::{log_line, only}; use super::{Dialed, Session}; use crate::connect::{ClusterExpectation, ConnectError, SystemConnector}; use crate::context::Connection; @@ -223,7 +223,7 @@ pub async fn github_check_in( }) .await?; if build.status != BuildStatus::Building || build.grant.is_some() { - return Err(conflict( + return Err(RpcError::conflict( "This build already checked in or is no longer wanted", )); } @@ -240,7 +240,7 @@ pub async fn github_check_in( BTreeMap::from([(id.service.clone(), commit.to_string())]), )? .remove(&id.service) - .ok_or_else(|| internal("The build has no fingerprint"))?; + .ok_or_else(|| RpcError::internal("The build has no fingerprint"))?; let sealed = sealed_record(&deployment, &id.service)?; let session = session(connections, expect).await?; let foreign = session.foreign(); @@ -257,7 +257,7 @@ pub async fn github_check_in( move |store| store.github_end(&id, Some(run_id), &end) }) .await?; - return Err(conflict(OLD_MINT_SKIPS)); + return Err(RpcError::conflict(OLD_MINT_SKIPS)); } Err(error) => { session.close().await; @@ -354,7 +354,7 @@ pub async fn github_rule_check_in( let (Some(ployz_core::Sealing::Held { public_key }), Some(attest)) = (contract.sealing, contract.attest) else { - return Err(conflict( + return Err(ployz_core::RpcError::conflict( "GitHub builds under an automatic deploy rule need a newer Ployz on your servers", )); }; @@ -407,7 +407,7 @@ pub async fn github_rule_grant( ) -> Result, RpcError> { let (id, rule, built) = rule_build(&store, &claims, &ask.commit.to_string()).await?; if ask.service != id.service { - return Err(conflict( + return Err(ployz_core::RpcError::conflict( "The grant asks for another Service than its build's", )); } @@ -417,7 +417,7 @@ pub async fn github_rule_grant( BTreeMap::from([(id.service.clone(), built.commit.to_string())]), )? .remove(&id.service) - .ok_or_else(|| internal("The build has no fingerprint"))?; + .ok_or_else(|| ployz_core::RpcError::internal("The build has no fingerprint"))?; let sealed = sealed_record(&deployment, &id.service)?; let session = session(connections, expect).await?; let foreign = session.foreign(); @@ -430,7 +430,7 @@ pub async fn github_rule_grant( .contains(ployz_core::BUILD_GRANT_RULE_CAPABILITY) }); if !advertises { - return Err(conflict( + return Err(ployz_core::RpcError::conflict( "GitHub builds under an automatic deploy rule need a newer Ployz on your servers", )); } @@ -456,7 +456,7 @@ pub async fn github_rule_grant( session.close().await; let (machine, minted) = minted?; let MintedKey::Holder { machine: identity } = minted.key else { - return Err(internal( + return Err(ployz_core::RpcError::internal( "The Machine returned a grant secret for a held grant", )); }; @@ -505,16 +505,16 @@ async fn rule_build( })?; let build = store.github_authorize(&id, &claims)?; if build.status != BuildStatus::Building || build.grant.is_some() { - return Err(conflict( + return Err(ployz_core::RpcError::conflict( "This build already checked in or is no longer wanted", )); } - let rule = store - .github_rule(&id)? - .ok_or_else(|| conflict("This build runs under no automatic deploy rule"))?; + let rule = store.github_rule(&id)?.ok_or_else(|| { + ployz_core::RpcError::conflict("This build runs under no automatic deploy rule") + })?; let built = store.github_input(&id)?; if built.commit.as_str() != commit { - return Err(conflict( + return Err(ployz_core::RpcError::conflict( "This run checked out another commit than the one this build pins", )); } @@ -548,7 +548,7 @@ fn sealed_record( .map(SealedRecord::of) .transpose() .map(Option::flatten) - .map_err(|error| internal(&error.to_string())) + .map_err(|error| RpcError::internal(error.to_string())) } /// Mint on the session's Machine, or `None` when GitHub can't take this build. @@ -603,7 +603,7 @@ fn minting(holder: Option, capabilities: &BTreeSet, sealed /// The one repository a runner's Build Grant may push into. fn repository(id: &GithubBuildId) -> Result { BuildGrantRepository::parse(format!("ployz-build/{}", id.service)) - .map_err(|_| internal("The Service has no Build Grant repository")) + .map_err(|_| RpcError::internal("The Service has no Build Grant repository")) } /// One batch of a runner's `ployz build --events` lines, from line `from`, and once @@ -650,12 +650,8 @@ pub async fn github_report( claims: GithubClaims, report: Value, ) -> Result { - let report: StepsReport = serde_json::from_value(report).map_err(|_| RpcError { - code: RpcErrorCode::InvalidArgument, - message: "The report is not Build Steps".to_owned(), - details: Value::Null, - cause: Vec::new(), - })?; + let report: StepsReport = serde_json::from_value(report) + .map_err(|_| RpcError::invalid_argument("The report is not Build Steps"))?; let mut lines: Vec = report .events .iter() @@ -663,12 +659,9 @@ pub async fn github_report( .collect(); let run_end = match (report.install_failed, report.platforms) { (Some(_), Some(_)) => { - return Err(RpcError { - code: RpcErrorCode::InvalidArgument, - message: "A report ends a build or says ployz didn't install, not both".to_owned(), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::invalid_argument( + "A report ends a build or says ployz didn't install, not both", + )); } // No final report: GitHub failed it, so the next Builder takes it. (Some(version), None) => { @@ -938,25 +931,13 @@ async fn session( expect: Option, ) -> Result { if connections.is_empty() { - return Err(RpcError { - code: RpcErrorCode::Unavailable, - message: "No Server is enrolled in this Organization".to_owned(), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::unavailable( + "No Server is enrolled in this Organization", + )); } Ok(super::dial(connections, Arc::new(SystemConnector::default()), expect).await?) } -fn conflict(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message: message.to_owned(), - details: Value::Null, - cause: Vec::new(), - } -} - #[cfg(test)] mod tests { use super::*; @@ -1079,9 +1060,9 @@ mod tests { }; let mint = ployz_core::MINT_BUILD_GRANT_CAPABILITY; let refused = |error| crate::connect::refused_by_enforcement(&error, mint); - assert!(refused(refusal(crate::connect::FORBIDDEN, mint))); + assert!(refused(refusal(ployz_core::FORBIDDEN_CODE, mint))); assert!(!refused(refusal( - crate::connect::FORBIDDEN, + ployz_core::FORBIDDEN_CODE, ployz_core::BUILD_CAPABILITY ))); assert!(!refused(refusal("conflict", mint))); diff --git a/core/crates/ployz/src/sdk/logs.rs b/core/crates/ployz/src/sdk/logs.rs index e44a2d6a5e..41d9e583a7 100644 --- a/core/crates/ployz/src/sdk/logs.rs +++ b/core/crates/ployz/src/sdk/logs.rs @@ -13,7 +13,7 @@ use tokio::sync::Mutex; use tokio_util::sync::CancellationToken; use ts_rs::TS; -use super::{Session, invalid_argument}; +use super::Session; use crate::connect::{Client, ConnectError}; use ployz_core::{InspectContainerRequest, RetainedEnvRequest}; use ployz_store::tools::{LineCap, LogExcerpt, LogText, SealedValues, Withheld}; @@ -124,7 +124,7 @@ impl Session { input: ContainerLogInput, ) -> Result { if !(-1..=1000).contains(&input.tail) { - return Err(invalid_argument("tail must be -1..1000".into())); + return Err(RpcError::invalid_argument("tail must be -1..1000")); } let client = self.client()?; let target = MachineTarget::from(&input.machine_id); @@ -169,7 +169,7 @@ impl Session { limit: input.limit, cursor: input.cursor, }; - request.validate().map_err(invalid_argument)?; + request.validate().map_err(RpcError::invalid_argument)?; let payload = op::LogHistory::into_request(request) .encode() .map_err(|error| crate::ui::rpc_error(RpcErrorCode::InvalidArgument, &error))?; @@ -212,12 +212,7 @@ impl LogHistoryStream { let payload = tokio::select! { () = self.cancel.cancelled() => return Ok(None), payload = tokio::time::timeout(ROW_DEADLINE, stream.message()) => payload - .map_err(|_| RpcError { - code: RpcErrorCode::Unavailable, - message: "the Server's Log Store stopped answering".into(), - details: serde_json::Value::Null, - cause: Vec::new(), - })? + .map_err(|_| RpcError::unavailable("the Server's Log Store stopped answering"))? .map_err(|error| RpcError::from(ConnectError::from(error)))?, }; let Some(payload) = payload else { @@ -247,9 +242,9 @@ impl Drop for LogHistoryStream { fn nanos(value: Option<&str>, field: &str) -> Result, RpcError> { value .map(|value| { - value - .parse() - .map_err(|_| invalid_argument(format!("{field} must be a decimal integer"))) + value.parse().map_err(|_| { + RpcError::invalid_argument(format!("{field} must be a decimal integer")) + }) }) .transpose() } @@ -259,12 +254,7 @@ fn history_row(payload: &OpaquePayload) -> Result, RpcError> .map_err(|error| crate::ui::rpc_error(RpcErrorCode::InvalidArgument, &error))? { HistoryRow::Heartbeat => Ok(None), - HistoryRow::Error(message) => Err(RpcError { - code: RpcErrorCode::Unavailable, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - }), + HistoryRow::Error(message) => Err(RpcError::unavailable(message)), row @ (HistoryRow::Container(_) | HistoryRow::Line { .. } | HistoryRow::Gap { .. } @@ -604,7 +594,7 @@ pub(crate) async fn process_tail( LogBody::Stdout(bytes) | LogBody::Stderr(bytes) => { frames.push(String::from_utf8_lossy(&bytes).into_owned()); } - LogBody::Error(message) => return Err(unavailable(message)), + LogBody::Error(message) => return Err(RpcError::unavailable(message)), LogBody::Heartbeat => {} } } @@ -622,22 +612,13 @@ async fn in_time( tokio::time::timeout(SERVICE_TAIL_READ, read) .await .map_err(|_| { - unavailable(format!( + RpcError::unavailable(format!( "{what} did not answer within {}s", SERVICE_TAIL_READ.as_secs() )) })? } -fn unavailable(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - } -} - /// The env that scrubs a Container's lines: Docker's, or the one its Machine's /// Log Store kept once the Machine answers that the Container is gone. `None` /// when neither answers, which withholds the lines. @@ -711,12 +692,7 @@ async fn newest_lines( } => newest.push((container_id, text)), HistoryRow::End { next } => cursor = next, HistoryRow::Error(message) => { - return Err(RpcError { - code: RpcErrorCode::Unavailable, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::unavailable(message)); } HistoryRow::Exit { exit_code, .. } => match &mut exit { None => { diff --git a/core/crates/ployz/src/sdk/metrics.rs b/core/crates/ployz/src/sdk/metrics.rs index ec526e4755..90c9a86811 100644 --- a/core/crates/ployz/src/sdk/metrics.rs +++ b/core/crates/ployz/src/sdk/metrics.rs @@ -10,7 +10,7 @@ use std::collections::BTreeMap; use futures_util::future::join_all; use ployz_core::{ ListMachinesRequest, MachineFailure, MachineId, MachineSuccess, MachineTarget, - QualifiedService, RpcError, RpcErrorCode, + QualifiedService, RpcError, metrics::{ MetricHistoryReply, MetricHistoryRequest, MetricPoints, MetricRange, MetricScope, ServicePoint, @@ -18,10 +18,9 @@ use ployz_core::{ op, }; use serde::{Deserialize, Serialize}; -use serde_json::Value; use ts_rs::TS; -use super::{Session, invalid_argument}; +use super::Session; use crate::connect::Client; /// Whose history to read: one Machine's host totals, or one Service summed @@ -105,7 +104,7 @@ pub async fn metric_history( range, step_nanos, }; - request.validate().map_err(invalid_argument)?; + request.validate().map_err(RpcError::invalid_argument)?; let machines = match machine { Some(machine_id) => Asked::one(machine_id), None => Asked::cluster(client).await?, @@ -161,7 +160,7 @@ impl Asked { } else { asked.unreachable.push(MachineFailure { machine_id: machine.machine.id, - error: unavailable("the Server is not up, so it was not asked"), + error: RpcError::unavailable("the Server is not up, so it was not asked"), }); } } @@ -186,7 +185,9 @@ fn merge( let MetricPoints::Service(points) = &reply.value.points else { unanswered.push(MachineFailure { machine_id: reply.machine_id, - error: unavailable("the Server answered with host totals, not a Service"), + error: RpcError::unavailable( + "the Server answered with host totals, not a Service", + ), }); continue; }; @@ -332,16 +333,7 @@ impl Total { fn nanos(value: &str, field: &str) -> Result { value .parse() - .map_err(|_| invalid_argument(format!("{field} must be a decimal integer"))) -} - -fn unavailable(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: message.into(), - details: Value::Null, - cause: Vec::new(), - } + .map_err(|_| RpcError::invalid_argument(format!("{field} must be a decimal integer"))) } #[cfg(test)] diff --git a/core/crates/ployz/src/sdk/metrics_tests.rs b/core/crates/ployz/src/sdk/metrics_tests.rs index 062fef894d..3e0f5f4684 100644 --- a/core/crates/ployz/src/sdk/metrics_tests.rs +++ b/core/crates/ployz/src/sdk/metrics_tests.rs @@ -6,7 +6,7 @@ use ployz_core::{ }, }; -use super::{MetricHistory, merge, unavailable}; +use super::{MetricHistory, merge}; const SINCE: i64 = 1_000_000_000_000; const MIN: i64 = 60_000_000_000; @@ -62,7 +62,7 @@ fn a_machine_that_times_out_is_named_and_the_rest_still_sum() { let fast = answered(MIN, vec![point(0, 2.0, 100, Some(1_000))]); let timeout = MachineFailure { machine_id: slow, - error: unavailable("deadline exceeded"), + error: ployz_core::RpcError::unavailable("deadline exceeded"), }; let history = merge(&request(), vec![fast.clone()], vec![timeout]); diff --git a/core/crates/ployz/src/sdk/preparation.rs b/core/crates/ployz/src/sdk/preparation.rs index f9970d524f..4a1ea08b27 100644 --- a/core/crates/ployz/src/sdk/preparation.rs +++ b/core/crates/ployz/src/sdk/preparation.rs @@ -7,7 +7,7 @@ use std::{ use super::prepare::BuildPreference; use crate::build::{BuildSpec, BuiltService, CapturedBuild, Recipe}; use ployz_core::{ - DeployIntent, RpcError, RpcErrorCode, ServiceName, + DeployIntent, RpcError, ServiceName, config::{BuildMethod, ServiceBuildConfig, ServiceSource}, }; use serde::{Deserialize, Serialize}; @@ -151,12 +151,7 @@ pub(crate) fn receipts( } fn invalid(message: impl ToString) -> RpcError { - RpcError { - code: RpcErrorCode::InvalidArgument, - message: message.to_string(), - details: Value::Null, - cause: Vec::new(), - } + RpcError::invalid_argument(message.to_string()) } /// Capture authorized checkouts as Builds. @@ -406,15 +401,11 @@ pub(crate) fn upload_needed(services: &[ServiceName]) -> RpcError { .map(ServiceName::as_str) .collect::>() .join(", "); - RpcError { - code: RpcErrorCode::NotFound, - message: format!( - "No upload or usable image for {names}: its image is gone, or its build inputs \ + RpcError::not_found(format!( + "No upload or usable image for {names}: its image is gone, or its build inputs \ changed. Upload its source again" - ), - details: json!({"preparation": Needed::UploadNeeded { services: services.to_vec() }}), - cause: Vec::new(), - } + )) + .with_details(json!({"preparation": Needed::UploadNeeded { services: services.to_vec() }})) } /// The uploaded Services `error` says need a new upload, if that's what it says. @@ -610,6 +601,7 @@ mod flow_tests; #[cfg(test)] mod tests { use super::*; + use ployz_core::RpcErrorCode; #[test] fn capture_orders_by_cloud_references() { let deployment = json!({ diff --git a/core/crates/ployz/src/sdk/prepare.rs b/core/crates/ployz/src/sdk/prepare.rs index 72a592bd1a..976732bca5 100644 --- a/core/crates/ployz/src/sdk/prepare.rs +++ b/core/crates/ployz/src/sdk/prepare.rs @@ -214,12 +214,9 @@ pub async fn select_build_machine( } let message = format!("no eligible Build Machine: {}", reasons.join("; ")); let details = serde_json::json!({"rejections": rejected, "reasons": shown}); - Err(ConnectError::Remote(RpcError { - code: RpcErrorCode::Unsupported, - message, - details, - cause: Vec::new(), - })) + Err(ConnectError::Remote( + RpcError::unsupported(message).with_details(details), + )) } use crate::{ diff --git a/core/crates/ployz/src/sdk/running.rs b/core/crates/ployz/src/sdk/running.rs index a5391e5767..735ce7cd23 100644 --- a/core/crates/ployz/src/sdk/running.rs +++ b/core/crates/ployz/src/sdk/running.rs @@ -2,12 +2,11 @@ use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; -use ployz_core::{RpcError, RpcErrorCode}; +use ployz_core::RpcError; use serde_json::Value; use tokio::sync::Mutex; use tokio_util::sync::CancellationToken; -use super::invalid_argument; use super::prepare::Progress; /// Most build output retained ahead of a slow consumer before it is dropped. @@ -152,15 +151,9 @@ impl> Running { .lock() .await .take() - .ok_or_else(|| E::from(invalid_argument("result already awaited".into())))?; - join.await.map_err(|_| { - E::from(RpcError { - code: RpcErrorCode::Internal, - message: "SDK call task failed".into(), - details: Value::Null, - cause: Vec::new(), - }) - })? + .ok_or_else(|| E::from(RpcError::invalid_argument("result already awaited")))?; + join.await + .map_err(|_| E::from(RpcError::internal("SDK call task failed")))? } } impl Drop for Running { @@ -179,7 +172,7 @@ mod tests { for n in 0..1000 { events.send((1, serde_json::json!({"n":n}))).unwrap(); } - Err::<(), _>(invalid_argument("fixture failure".into())) + Err::<(), _>(RpcError::invalid_argument("fixture failure")) }); let running = Running { cancel: CancellationToken::new(), diff --git a/core/crates/ployz/src/sdk/store_call.rs b/core/crates/ployz/src/sdk/store_call.rs index 865abfa54d..7b19bd285d 100644 --- a/core/crates/ployz/src/sdk/store_call.rs +++ b/core/crates/ployz/src/sdk/store_call.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use std::time::Duration; -use ployz_core::{RpcError, RpcErrorCode}; +use ployz_core::RpcError; use ployz_store::ConfigStore; use tokio::sync::Semaphore; @@ -40,8 +40,8 @@ async fn bounded( ) -> Result { let permit = tokio::time::timeout(queue, permits.acquire()) .await - .map_err(|_| unavailable("The Config Store is busy; retry"))? - .map_err(|_| unavailable("The Config Store is closed"))?; + .map_err(|_| RpcError::unavailable("The Config Store is busy; retry"))? + .map_err(|_| RpcError::unavailable("The Config Store is closed"))?; // The work holds its turn until it ends, even after its caller stops waiting, // so no more than the bound ever runs at once. let work = tokio::task::spawn_blocking(move || { @@ -51,23 +51,15 @@ async fn bounded( tokio::time::timeout(run, work) .await .map_err(|_| { - unavailable("The Config Store did not answer in time; the outcome is unknown") + RpcError::unavailable("The Config Store did not answer in time; the outcome is unknown") })? - .map_err(|_| unavailable("The Config Store call stopped unexpectedly"))? -} - -fn unavailable(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: message.to_owned(), - details: serde_json::Value::Null, - cause: Vec::new(), - } + .map_err(|_| RpcError::unavailable("The Config Store call stopped unexpectedly"))? } #[cfg(test)] mod tests { use super::*; + use ployz_core::RpcErrorCode; #[tokio::test] async fn a_call_waiting_past_its_turn_is_busy() { diff --git a/core/crates/ployz/src/sdk/store_runner.rs b/core/crates/ployz/src/sdk/store_runner.rs index 31b2e09a12..28da0f7e7c 100644 --- a/core/crates/ployz/src/sdk/store_runner.rs +++ b/core/crates/ployz/src/sdk/store_runner.rs @@ -251,9 +251,7 @@ pub async fn observe_volumes_expecting( expect: Option, ) -> Result { if connections.is_empty() { - return Err(super::invalid_argument( - "connections must not be empty".into(), - )); + return Err(RpcError::invalid_argument("connections must not be empty")); } let servers: Vec = connections .iter() @@ -310,9 +308,7 @@ pub async fn observe_running_expecting( expect: Option, ) -> Result { if connections.is_empty() { - return Err(super::invalid_argument( - "connections must not be empty".into(), - )); + return Err(RpcError::invalid_argument("connections must not be empty")); } let servers: Vec = connections .iter() @@ -1059,7 +1055,7 @@ impl Run { (status, None, Ok(receipt)) } Ok(BuildOutcome::Queued) => { - let error = internal("No Server started the build"); + let error = RpcError::internal("No Server started the build"); ( BuildStatus::Failed, Some(crate::ui::row(&error)), @@ -1875,15 +1871,6 @@ async fn remove_volumes( .unwrap_or_default() } -pub(super) fn internal(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Internal, - message: message.to_owned(), - details: serde_json::Value::Null, - cause: Vec::new(), - } -} - #[cfg(test)] #[path = "store_runner_tests.rs"] mod tests; diff --git a/core/crates/ployz/src/sdk/store_runner_tests.rs b/core/crates/ployz/src/sdk/store_runner_tests.rs index 4944548409..547d4ccf0c 100644 --- a/core/crates/ployz/src/sdk/store_runner_tests.rs +++ b/core/crates/ployz/src/sdk/store_runner_tests.rs @@ -643,7 +643,7 @@ async fn a_queued_build_says_so_in_its_log_while_it_waits() { ployz_build::Progress::Stage(ployz_build::Stage::Queued), )); let _ = released.await; - Err(internal("released")) + Err(RpcError::internal("released")) }); let log = || { run.store @@ -720,7 +720,7 @@ async fn a_build_reports_the_server_it_runs_on_by_name() { rejections: Vec::new(), }, ))); - Err(internal("the build failed")) + Err(RpcError::internal("the build failed")) }); assert!(run.follow(&target, &running).await.unwrap().is_err()); let build = run diff --git a/core/crates/ployz/src/sealed_compat.rs b/core/crates/ployz/src/sealed_compat.rs index 0262d858ff..65cddedf2c 100644 --- a/core/crates/ployz/src/sealed_compat.rs +++ b/core/crates/ployz/src/sealed_compat.rs @@ -5,8 +5,8 @@ use std::borrow::Cow; use ployz_core::{ BUILD_SEALED_CAPABILITY, ContainerWant, ENV_SEALED_CAPABILITY, EnvValue, - MANAGEMENT_SCOPE_CAPABILITY, MachineObservation, MachineTarget, RpcError, RpcErrorCode, - RunTarget, config::BuildMethod, machine_matches_target, + MANAGEMENT_SCOPE_CAPABILITY, MachineObservation, MachineTarget, RpcError, RunTarget, + config::BuildMethod, machine_matches_target, }; use crate::{cluster::Client, connect::ConnectError}; @@ -45,7 +45,7 @@ pub(crate) async fn admit( if !machines.iter().any(|observation| { !observation.unarrived() && machine_matches_target(&observation.machine, target) }) { - return Err(unsupported(format!( + return Err(RpcError::unsupported(format!( "{target} shows no evidence it can open Sealed values; refusing to send them" ))); } @@ -61,7 +61,7 @@ pub(crate) async fn admit( if behind.is_empty() { return Ok(()); } - Err(unsupported(format!( + Err(RpcError::unsupported(format!( "{} cannot open Sealed values; upgrade every Machine before deploying them", behind.join(", ") ))) @@ -122,7 +122,7 @@ pub(crate) fn builders<'a>( ), } }; - let mut error = unsupported(reason.clone()); + let mut error = RpcError::unsupported(reason.clone()); error.details = serde_json::json!({ "rejections": {"cannot open sealed values": behind.len()}, "reasons": [reason], @@ -130,15 +130,6 @@ pub(crate) fn builders<'a>( Err(ConnectError::Remote(error)) } -fn unsupported(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::Unsupported, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - } -} - #[cfg(test)] #[path = "sealed_compat_tests.rs"] pub(crate) mod tests; diff --git a/core/crates/ployz/src/sealing_backfill.rs b/core/crates/ployz/src/sealing_backfill.rs index 2dac548548..5302522ddd 100644 --- a/core/crates/ployz/src/sealing_backfill.rs +++ b/core/crates/ployz/src/sealing_backfill.rs @@ -12,12 +12,10 @@ use futures_util::future::join_all; use ployz_core::{ DescribeContractRequest, GENERATE_SEALING_KEY_CAPABILITY, GenerateSealingKeyRequest, INSTALL_SEALING_KEY_CAPABILITY, InspectRequest, InstallSealingKeyRequest, LocalMachinePhase, - MachineId, MachineTarget, RpcError, RpcErrorCode, SHARE_SEALING_KEY_CAPABILITY, - SealedSealingKey, Sealing, SealingPublicKey, SealingRefusal, ShareSealingKeyRequest, - WireGuardPublicKey, op, + MachineId, MachineTarget, RpcError, SHARE_SEALING_KEY_CAPABILITY, SealedSealingKey, Sealing, + SealingPublicKey, SealingRefusal, ShareSealingKeyRequest, WireGuardPublicKey, op, }; use serde::{Deserialize, Serialize}; -use serde_json::Value; use ts_rs::TS; use crate::connect::{Client, rpc_error}; @@ -358,12 +356,7 @@ async fn carry( wireguard_key: WireGuardPublicKey, public_key: SealingPublicKey, ) -> MachineOutcome { - let mut last = RpcError { - code: RpcErrorCode::Unsupported, - message: "no Machine that holds the Sealing Key can share it".into(), - details: Value::Null, - cause: Vec::new(), - }; + let mut last = RpcError::unsupported("no Machine that holds the Sealing Key can share it"); for &source in sources { let sealed = match client.share(source, recipient, wireguard_key).await { Ok(sealed) => sealed, diff --git a/core/crates/ployz/src/ui/error.rs b/core/crates/ployz/src/ui/error.rs index 019c3463d4..4abd5edc6b 100644 --- a/core/crates/ployz/src/ui/error.rs +++ b/core/crates/ployz/src/ui/error.rs @@ -211,12 +211,7 @@ pub(crate) fn retrying(operation: &str, error: Chain<'_>, seconds: u64) { } pub(crate) fn rpc_error(code: ployz_core::RpcErrorCode, error: Chain<'_>) -> ployz_core::RpcError { - ployz_core::RpcError { - code, - message: inline(error), - details: Value::Null, - cause: Vec::new(), - } + ployz_core::RpcError::new(code, inline(error)) } /// `text` as a failure's first line: a sentence, ending with a period. diff --git a/core/crates/ployz/src/ui/result.rs b/core/crates/ployz/src/ui/result.rs index 3797390fe4..f70ae9a5d9 100644 --- a/core/crates/ployz/src/ui/result.rs +++ b/core/crates/ployz/src/ui/result.rs @@ -353,7 +353,7 @@ impl Gaps { return (refusal.naming(name).to_string(), None); } // It answered, refusing the caller's access: its reason, never "did not answer". - if failure.error.code.as_str() == crate::connect::FORBIDDEN { + if failure.error.code.as_str() == ployz_core::FORBIDDEN_CODE { let next = failure .error .details diff --git a/core/crates/ployzd/src/docker/stream.rs b/core/crates/ployzd/src/docker/stream.rs index 814891b3e6..d6ef1568bd 100644 --- a/core/crates/ployzd/src/docker/stream.rs +++ b/core/crates/ployzd/src/docker/stream.rs @@ -12,7 +12,6 @@ use ployz_core::{ ContainerId, ExecRequestFrame, ExecResponseFrame, LogMetadata, LogOrigin, LogsOptions, MachineId, MachineName, OpaquePayload, RpcError, RpcErrorCode, }; -use serde_json::Value; use tokio::{io::AsyncWriteExt, sync::mpsc}; use tonic::{Status, Streaming}; @@ -380,12 +379,7 @@ async fn send_exec_error( ) -> Result<(), Status> { send_exec( sender, - ExecResponseFrame::Error(RpcError { - code: RpcErrorCode::Internal, - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - }), + ExecResponseFrame::Error(RpcError::internal(error.to_string())), ) .await } diff --git a/core/crates/ployzd/src/machine/local_machine.rs b/core/crates/ployzd/src/machine/local_machine.rs index a6dd08ece6..037cadf9fa 100644 --- a/core/crates/ployzd/src/machine/local_machine.rs +++ b/core/crates/ployzd/src/machine/local_machine.rs @@ -846,12 +846,7 @@ impl LocalMachine { .await; if !leftovers.is_empty() { return Ok(Enrolled::Refused { - error: RpcError { - code: RpcErrorCode::Conflict, - message: leftovers_refusal(&leftovers), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error: RpcError::conflict(leftovers_refusal(&leftovers)), now: self.record().now(), }); } @@ -862,12 +857,7 @@ impl LocalMachine { EnrollPlan::Replay(machine) => machine, EnrollPlan::Conflict(now) => { return Ok(Enrolled::Refused { - error: RpcError { - code: RpcErrorCode::Conflict, - message: enroll_conflict(&now.phase).into(), - details: serde_json::Value::Null, - cause: Vec::new(), - }, + error: RpcError::conflict(enroll_conflict(&now.phase)), now, }); } @@ -1522,12 +1512,7 @@ fn leave_response(owner: &RecordOwner, reset_warning: Option) -> Left { } fn refusal(code: RpcErrorCode, message: &str) -> RpcError { - RpcError { - code, - message: message.into(), - details: serde_json::Value::Null, - cause: Vec::new(), - } + RpcError::new(code, message) } #[cfg(test)] diff --git a/core/crates/ployzd/src/machine_api/local.rs b/core/crates/ployzd/src/machine_api/local.rs index ecf88538e4..3cdd73c3a8 100644 --- a/core/crates/ployzd/src/machine_api/local.rs +++ b/core/crates/ployzd/src/machine_api/local.rs @@ -14,7 +14,6 @@ use ployz_core::{ RoutesRead, Rpc, RpcError, RpcErrorCode, RpcRequestBody, RpcResponse, SelectEndpointRequest, WantedCopy, op, }; -use serde_json::Value; use tonic::{Request, Response, Status}; use crate::{ @@ -185,12 +184,12 @@ impl MachineService { fn replicated(&self) -> Result<&ReplicatedStore, RpcError> { self.local .replicated() - .map_err(|_| unavailable("Cluster store is not available")) + .map_err(|_| RpcError::unavailable("Cluster store is not available")) } fn ready_replicated(&self) -> Result<&ReplicatedStore, RpcError> { if self.local_record().phase() != LocalMachinePhase::Participating { - return Err(unavailable("Machine is not participating")); + return Err(RpcError::unavailable("Machine is not participating")); } self.replicated() } @@ -200,17 +199,14 @@ impl MachineService { async fn select(&self, request: SelectEndpointRequest) -> Result { let SelectEndpointRequest { peer, endpoint } = request; if peer == self.local_record().id() { - return Err(RpcError { - code: RpcErrorCode::InvalidArgument, - message: "a Server does not choose an endpoint for itself".into(), - details: Value::Null, - cause: Vec::new(), - }); + return Err(RpcError::invalid_argument( + "a Server does not choose an endpoint for itself", + )); } let chooser = self .endpoint_chooser .as_ref() - .ok_or_else(|| unavailable("WireGuard is not running on this Server"))?; + .ok_or_else(|| RpcError::unavailable("WireGuard is not running on this Server"))?; let (applied, outcome) = tokio::sync::oneshot::channel(); chooser .send(EndpointChoice { @@ -219,7 +215,7 @@ impl MachineService { applied, }) .await - .map_err(|_| unavailable("WireGuard is not running on this Server"))?; + .map_err(|_| RpcError::unavailable("WireGuard is not running on this Server"))?; match outcome.await { Ok(Ok(())) => Ok(EndpointSelected { peer, endpoint }), Ok(Err(refused @ ChoiceRefused::UnknownPeer(_))) => { @@ -228,7 +224,9 @@ impl MachineService { Ok(Err(refused @ ChoiceRefused::WireGuard)) => { Err(RpcError::caused(RpcErrorCode::Internal, &refused)) } - Err(_) => Err(unavailable("WireGuard is not running on this Server")), + Err(_) => Err(RpcError::unavailable( + "WireGuard is not running on this Server", + )), } } @@ -256,7 +254,7 @@ impl MachineService { fn containers(&self) -> Result<&ContainerRuntime, RpcError> { self.local .containers() - .ok_or_else(|| unavailable("Docker is not available")) + .ok_or_else(|| RpcError::unavailable("Docker is not available")) } fn require_joined(&self) -> Result<(), RpcError> { @@ -265,7 +263,7 @@ impl MachineService { LocalMachinePhase::Uninitialized | LocalMachinePhase::Resetting | LocalMachinePhase::Unrecognized(_) => { - Err(unavailable("Machine is not participating")) + Err(RpcError::unavailable("Machine is not participating")) } } } @@ -974,12 +972,7 @@ impl MachineRpc for MachineService { ) -> Result, Status> { let request = expect::(request)?; if request.image.is_empty() { - return respond(RpcError { - code: RpcErrorCode::InvalidArgument, - message: "image is required".into(), - details: Value::Null, - cause: Vec::new(), - }); + return respond(RpcError::invalid_argument("image is required")); } finish(self.local.pull_image(request).await) } @@ -990,12 +983,7 @@ impl MachineRpc for MachineService { ) -> Result, Status> { let request = expect::(request)?; if request.pull.image().is_empty() { - return respond(RpcError { - code: RpcErrorCode::InvalidArgument, - message: "image is required".into(), - details: Value::Null, - cause: Vec::new(), - }); + return respond(RpcError::invalid_argument("image is required")); } finish(self.local.pull_image_from_machine(request).await) } @@ -1023,12 +1011,9 @@ impl MachineRpc for MachineService { let admitted = self.admit_rule_grant(grant, request.holder.as_ref(), request.sealed.as_ref()); let Some(admitted) = admitted.await else { - return respond(RpcError { - code: RpcErrorCode::Unavailable, - message: "This Server cannot reach GitHub to check a rule build".into(), - details: Value::Null, - cause: Vec::new(), - }); + return respond(RpcError::unavailable( + "This Server cannot reach GitHub to check a rule build", + )); }; match admitted { Ok(admitted) => Some(admitted), @@ -1096,15 +1081,10 @@ impl MachineRpc for MachineService { let material = match material { Ok(material) => material, Err(error) => { - return respond(RpcError { - code: RpcErrorCode::InvalidArgument, - message: format!( - "refused Certificate Material for {hostname}: {error}", - error = ployz_core::error_chain::inline(&error), - ), - details: Value::Null, - cause: Vec::new(), - }); + return respond(RpcError::invalid_argument(format!( + "refused Certificate Material for {hostname}: {error}", + error = ployz_core::error_chain::inline(&error), + ))); } }; replicated @@ -1329,17 +1309,21 @@ fn local_failure(error: LocalMachineError) -> Result { LocalMachineError::Store(error) => Ok(store_error(error)), LocalMachineError::JoinRecord(_) => Err(internal_response(error)), LocalMachineError::ApprovalRecord(_) => Err(internal_response(error)), - LocalMachineError::NotParticipating => Ok(unavailable("Machine is not participating")), + LocalMachineError::NotParticipating => { + Ok(RpcError::unavailable("Machine is not participating")) + } LocalMachineError::ClusterStoreUnavailable => { - Ok(unavailable("Cluster store is not available")) + Ok(RpcError::unavailable("Cluster store is not available")) } LocalMachineError::ClusterNetworkUnpublished | LocalMachineError::ClusterStoreEmpty => { - Ok(unavailable(&error.to_string())) + Ok(RpcError::unavailable(error.to_string())) } LocalMachineError::ClusterUnavailable => { Err(Status::unavailable("Cluster is not available")) } - LocalMachineError::DockerUnavailable => Ok(unavailable("Docker is not available")), + LocalMachineError::DockerUnavailable => { + Ok(RpcError::unavailable("Docker is not available")) + } LocalMachineError::PasskeyIsDevice(_) | LocalMachineError::DeviceIsPasskey(_) | LocalMachineError::CredentialHeld { .. } => { @@ -1352,12 +1336,9 @@ fn local_failure(error: LocalMachineError) -> Result { LocalMachineError::Enrollment(error) => { Ok(RpcError::caused(RpcErrorCode::Conflict, &error)) } - LocalMachineError::EmptyUpdate => Ok(RpcError { - code: RpcErrorCode::InvalidArgument, - message: "at least one Machine update is required".into(), - details: Value::Null, - cause: Vec::new(), - }), + LocalMachineError::EmptyUpdate => Ok(RpcError::invalid_argument( + "at least one Machine update is required", + )), LocalMachineError::RecordOwner(error) => Err(ployz_core::rpc::caused_status( tonic::Code::Internal, &error, @@ -1374,19 +1355,13 @@ fn local_failure(error: LocalMachineError) -> Result { Ok(RpcError::caused(RpcErrorCode::InvalidArgument, &error)) } LocalMachineError::PrivateAccessRouterSpec | LocalMachineError::PrivateAccessKey(_) => { - Ok(RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - }) + Ok(RpcError::invalid_argument(error.to_string())) + } + LocalMachineError::PrivateAccessKeyMissing => { + Ok(RpcError::invalid_argument(error.to_string()).with_details( + serde_json::json!({ "capability": ployz_core::SET_PRIVATE_ACCESS_CAPABILITY }), + )) } - LocalMachineError::PrivateAccessKeyMissing => Ok(RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: serde_json::json!({ "capability": ployz_core::SET_PRIVATE_ACCESS_CAPABILITY }), - cause: Vec::new(), - }), LocalMachineError::PrivateAccessKeyFile(_) | LocalMachineError::PrivateAccessState(_) | LocalMachineError::PrivateAccessRouterStays { .. } => { @@ -1399,25 +1374,15 @@ fn local_failure(error: LocalMachineError) -> Result { // Both ask the caller for the doors route, so its capability is named. LocalMachineError::UnsupportedDoors | LocalMachineError::Docker(crate::docker::Error::IngressHolderNotRunning) => { - Ok(RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: serde_json::json!({ "capability": ployz_core::SET_INGRESS_DOORS_CAPABILITY }), - cause: Vec::new(), - }) + Ok(RpcError::invalid_argument(error.to_string()).with_details( + serde_json::json!({ "capability": ployz_core::SET_INGRESS_DOORS_CAPABILITY }), + )) } - LocalMachineError::TunnelTokenMissing => Ok(RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: serde_json::json!({ "capability": ployz_core::SET_INGRESS_TUNNEL_CAPABILITY }), - cause: Vec::new(), - }), - LocalMachineError::TunnelToken(_) => Ok(RpcError { - code: RpcErrorCode::InvalidArgument, - message: error.to_string(), - details: Value::Null, - cause: Vec::new(), - }), + LocalMachineError::TunnelTokenMissing => Ok(RpcError::invalid_argument(error.to_string()) + .with_details( + serde_json::json!({ "capability": ployz_core::SET_INGRESS_TUNNEL_CAPABILITY }), + )), + LocalMachineError::TunnelToken(_) => Ok(RpcError::invalid_argument(error.to_string())), LocalMachineError::DoorNotRunning { .. } | LocalMachineError::TunnelTokenFile(_) => { Ok(RpcError::caused(RpcErrorCode::Internal, &error)) } @@ -1425,18 +1390,10 @@ fn local_failure(error: LocalMachineError) -> Result { LocalMachineError::StoragePreparation(error) | LocalMachineError::FenceRefused(error) => { Ok(error) } - LocalMachineError::Cleanup(message) => Ok(RpcError { - code: RpcErrorCode::Internal, - message, - details: Value::Null, - cause: Vec::new(), - }), - LocalMachineError::Admission(crate::mutation::Error::Busy) => Ok(RpcError { - code: RpcErrorCode::Conflict, - message: "a Ployz installation or upgrade is active".into(), - details: Value::Null, - cause: Vec::new(), - }), + LocalMachineError::Cleanup(message) => Ok(RpcError::internal(message)), + LocalMachineError::Admission(crate::mutation::Error::Busy) => Ok(RpcError::conflict( + "a Ployz installation or upgrade is active", + )), LocalMachineError::Admission(crate::mutation::Error::Io(error)) => Err( ployz_core::rpc::caused_status(tonic::Code::Internal, &error), ), @@ -1465,10 +1422,7 @@ fn sealing_error(error: SealingError) -> RpcError { (RpcErrorCode::InvalidArgument, Refusal::SealedAnother) } }; - RpcError { - details: refusal.details(), - ..RpcError::caused(code, &error) - } + RpcError::caused(code, &error).with_details(refusal.details()) } /// Refusals name themselves in `details` so the CLI tells a wrong code from a @@ -1488,10 +1442,7 @@ fn recovery_error(error: RecoveryError) -> RpcError { RecoveryError::WrongCode => (RpcErrorCode::InvalidArgument, Refusal::WrongCode), RecoveryError::HeldAnother { .. } => (RpcErrorCode::Conflict, Refusal::HeldAnother), }; - RpcError { - details: refusal.details(), - ..RpcError::caused(code, &error) - } + RpcError::caused(code, &error).with_details(refusal.details()) } #[expect( @@ -1542,18 +1493,9 @@ fn finish( } } -fn unavailable(message: &str) -> RpcError { - RpcError { - code: RpcErrorCode::Unavailable, - message: message.into(), - details: Value::Null, - cause: Vec::new(), - } -} - fn store_error(error: StoreError) -> RpcError { let code = match error { - StoreError::JoinRefused(_) => RpcErrorCode::from("forbidden"), + StoreError::JoinRefused(_) => RpcErrorCode::from(ployz_core::FORBIDDEN_CODE), StoreError::AlreadyResetting | StoreError::AlreadyInitialized | StoreError::NotParticipating diff --git a/core/crates/ployzd/src/machine_api/local_volume_copy.rs b/core/crates/ployzd/src/machine_api/local_volume_copy.rs index b760679310..7beec208c0 100644 --- a/core/crates/ployzd/src/machine_api/local_volume_copy.rs +++ b/core/crates/ployzd/src/machine_api/local_volume_copy.rs @@ -5,8 +5,8 @@ use ployz_core::{ ContainerKind, ContainerLocator, ContainerNow, ContainerObservation, ContainerRefusalReason, ContainerRuntimeObservation, ContainerWant, CopyPlan, CopyRefusal, CopyRotation, CopyTransition, DockerVolumeName, EnsuredContainer, HealthVerdict, OpaquePayload, RpcError, - RpcErrorCode, RunRotation, RunTarget, SetVolumeCopyRefused, SetVolumeCopyReply, - SetVolumeCopyRequest, Start, VolumeCopy, VolumeGen, WantedCopy, + RunRotation, RunTarget, SetVolumeCopyRefused, SetVolumeCopyReply, SetVolumeCopyRequest, Start, + VolumeCopy, VolumeGen, WantedCopy, }; use tonic::{Response, Status}; @@ -520,10 +520,5 @@ fn refusal(refused: SetVolumeCopyRefused, name: &DockerVolumeName) -> LocalMachi } fn not_routed(message: String) -> RpcError { - RpcError { - code: RpcErrorCode::Conflict, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - } + RpcError::conflict(message) } diff --git a/core/crates/ployzd/src/management/build_grant.rs b/core/crates/ployzd/src/management/build_grant.rs index 923384a4ec..3169454b33 100644 --- a/core/crates/ployzd/src/management/build_grant.rs +++ b/core/crates/ployzd/src/management/build_grant.rs @@ -169,31 +169,31 @@ impl Mint { .and_then(|bytes| <[u8; 32]>::try_from(bytes).ok()) .and_then(|bytes| PublicKey::from_bytes(&bytes).ok()) .ok_or_else(|| { - refused( + RpcError::new( RpcErrorCode::InvalidArgument, - "the Build Grant holder is not a public key".into(), + "the Build Grant holder is not a public key", ) })?, ), }; if matches!(key, Key::Made(_)) && (request.sealed.is_some() || admitted.is_some()) { - return Err(refused( + return Err(RpcError::new( RpcErrorCode::InvalidArgument, - "a Build Grant carrying Sealed values needs a holder".into(), + "a Build Grant carrying Sealed values needs a holder", )); } let key_dir = || { key_dir.ok_or_else(|| { - refused( + RpcError::new( RpcErrorCode::Conflict, - "this Machine has no Sealing Key to open the Build's Sealed values".into(), + "this Machine has no Sealing Key to open the Build's Sealed values", ) }) }; if let Some(record) = &request.sealed { record .open_for_build(Some(key_dir()?)) - .map_err(|error| refused(RpcErrorCode::Conflict, error.to_string()))?; + .map_err(|error| RpcError::new(RpcErrorCode::Conflict, error.to_string()))?; } let (release, minted_by) = match (admitted, request.sealed) { (None, None) => (None, Minter::Operator), @@ -269,9 +269,9 @@ impl BuildGrants { let mut grants = self.grants.lock().expect("grant registry lock"); grants.retain(|_, grant| grant.expires > Instant::now()); if grants.contains_key(&id) { - return Err(refused( + return Err(RpcError::new( RpcErrorCode::Conflict, - "this holder already holds a live Build Grant".into(), + "this holder already holds a live Build Grant", )); } grants.insert(id, grant); @@ -293,14 +293,7 @@ impl BuildGrants { .expect("grant registry lock") .get(id) .filter(|grant| grant.expires > Instant::now()) - .ok_or_else(|| RpcError { - code: RpcErrorCode::NotFound, - message: - "this Machine holds no such Build Grant; it expired or the daemon restarted" - .into(), - details: serde_json::Value::Null, - cause: Vec::new(), - })?, + .ok_or_else(|| RpcError::not_found("this Machine holds no such Build Grant; it expired or the daemon restarted"))?, ); let pushed = match &*grant.push.lock().expect("grant push lock") { Push::Pushed(digest) => Some(digest.clone()), @@ -345,15 +338,6 @@ pub(crate) async fn mint_for( )) } -const fn refused(code: RpcErrorCode, message: String) -> RpcError { - RpcError { - code, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - } -} - fn grant_id(key: &[u8; 32]) -> BuildGrantId { BuildGrantId::parse(hex::encode(key)).expect("hex of 32 bytes is a Build Grant ID") } diff --git a/core/crates/ployzd/src/volume_plugin/capacity.rs b/core/crates/ployzd/src/volume_plugin/capacity.rs index 2189676d68..6a0f333946 100644 --- a/core/crates/ployzd/src/volume_plugin/capacity.rs +++ b/core/crates/ployzd/src/volume_plugin/capacity.rs @@ -20,14 +20,13 @@ fn unknown(error: impl std::fmt::Display) -> ployz_core::RpcError { fn storage_error(error: super::VolumeError) -> ployz_core::RpcError { match error { super::VolumeError::Capacity(error) => error.into_rpc_error(), - error => ployz_core::RpcError { - code: ployz_core::RpcErrorCode::Internal, - message: "Storage preparation failed".into(), - details: serde_json::json!({"code": "storage_preparation_failed"}), - cause: std::iter::once(error.to_string()) - .chain(ployz_core::error_chain::causes(&error)) - .collect(), - }, + error => ployz_core::RpcError::internal("Storage preparation failed") + .with_details(serde_json::json!({"code": "storage_preparation_failed"})) + .with_causes( + std::iter::once(error.to_string()) + .chain(ployz_core::error_chain::causes(&error)) + .collect(), + ), } } diff --git a/core/crates/ployzd/src/volume_plugin/inspect.rs b/core/crates/ployzd/src/volume_plugin/inspect.rs index 819e8baa77..d06990a964 100644 --- a/core/crates/ployzd/src/volume_plugin/inspect.rs +++ b/core/crates/ployzd/src/volume_plugin/inspect.rs @@ -2,8 +2,8 @@ use axum::{Json, extract::State}; use ployz_core::{ - InspectVolumeCopyRequest, Pull, PullState, RootMark, RpcError, RpcErrorCode, SlotId, SlotRole, - Snapshot, SnapshotGuid, SnapshotName, VolumeCopy, VolumeCopyView, WriterEpoch, + InspectVolumeCopyRequest, Pull, PullState, RootMark, RpcError, SlotId, SlotRole, Snapshot, + SnapshotGuid, SnapshotName, VolumeCopy, VolumeCopyView, WriterEpoch, }; use ployzd::machine_pool::MachinePool; @@ -109,12 +109,7 @@ pub(super) fn answering(mut copy: Option, failure: String) -> Option pub(super) fn internal(error: VolumeError) -> RpcError { match error { VolumeError::Capacity(error) => error.into_rpc_error(), - VolumeError::Message(message) => RpcError { - code: RpcErrorCode::Internal, - message, - details: serde_json::Value::Null, - cause: Vec::new(), - }, + VolumeError::Message(message) => RpcError::internal(message), } } From a0215aa11bd6a3ea109589657f379284281fdb02 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 13:33:14 +0000 Subject: [PATCH 2/7] Read a Build's refusal by enforcement from its capability, not its sentence --- core/crates/ployz/src/build/mod.rs | 1 + core/crates/ployz/src/build/remote.rs | 138 +++++++++++++++++++------- core/crates/ployz/src/sdk/prepare.rs | 5 +- 3 files changed, 108 insertions(+), 36 deletions(-) diff --git a/core/crates/ployz/src/build/mod.rs b/core/crates/ployz/src/build/mod.rs index 55f9d9ba90..1d25df274b 100644 --- a/core/crates/ployz/src/build/mod.rs +++ b/core/crates/ployz/src/build/mod.rs @@ -27,6 +27,7 @@ mod platforms; pub use platforms::placement_platforms; mod remote; pub use local::LocalImage; +pub(crate) use remote::CapabilityCheckError; mod reuse; pub(crate) use reuse::{holder, image_stores, runs_everywhere}; mod sealed; diff --git a/core/crates/ployz/src/build/remote.rs b/core/crates/ployz/src/build/remote.rs index 236edf4146..c1bad8a4fd 100644 --- a/core/crates/ployz/src/build/remote.rs +++ b/core/crates/ployz/src/build/remote.rs @@ -25,7 +25,7 @@ impl Client { machine_id: MachineId, targets: &[ployz_build::Target], cancellation: &CancellationToken, - ) -> Result<(), remote::InputError> { + ) -> Result<(), CapabilityCheckError> { let mut admitted = match open_and_admit( self, Input::Check(targets.to_vec()), @@ -37,30 +37,40 @@ impl Client { .await { Ok(admitted) => admitted, - Err(Outcome::Failed { message, .. } | Outcome::Unknown { message, .. }) => { - return Err(message.into()); + Err(Refused { + outcome: Outcome::Failed { message, .. } | Outcome::Unknown { message, .. }, + by_enforcement, + }) => { + return Err(if by_enforcement { + CapabilityCheckError::RefusedByEnforcement(message) + } else { + remote::InputError::from(message).into() + }); } - Err( - Outcome::CapabilitiesChecked { .. } - | Outcome::Images { .. } - | Outcome::Validated { .. } - | Outcome::Published { .. }, - ) => unreachable!("admission returns only terminal failures"), + Err(Refused { + outcome: + Outcome::CapabilitiesChecked { .. } + | Outcome::Images { .. } + | Outcome::Validated { .. } + | Outcome::Published { .. }, + .. + }) => unreachable!("admission returns only terminal failures"), }; // Keep the admitted sender alive: closing it would cancel the worker check. let deadline = tokio::time::Instant::now() + admitted.active_timeout + Duration::from_secs(70); + let failed = |message: &str| CapabilityCheckError::from(remote::InputError::from(message)); tokio::select! { biased; - () = cancellation.cancelled() => Err("Build selection cancelled".into()), - () = tokio::time::sleep_until(deadline) => Err("Build capability deadline expired".into()), + () = cancellation.cancelled() => Err(failed("Build selection cancelled")), + () = tokio::time::sleep_until(deadline) => Err(failed("Build capability deadline expired")), response = admitted.responses.message() => { let payload = response.map_err(|error| remote::InputError::from(error.to_string()))? - .ok_or_else(|| remote::InputError::from("Build capability response ended early"))?; + .ok_or_else(|| failed("Build capability response ended early"))?; match remote::decode::(&payload)? { Event::Finished(Outcome::CapabilitiesChecked { machine_id: actual }) if actual == machine_id => Ok(()), - Event::Finished(Outcome::Failed { message, .. } | Outcome::Unknown { message, .. }) => Err(message.into()), - Event::Admitted { .. } | Event::Progress(_) | Event::Finished(_) => Err("invalid Build capability response".into()), + Event::Finished(Outcome::Failed { message, .. } | Outcome::Unknown { message, .. }) => Err(remote::InputError::from(message).into()), + Event::Admitted { .. } | Event::Progress(_) | Event::Finished(_) => Err(failed("invalid Build capability response")), } } } @@ -131,6 +141,31 @@ struct Admitted { active_timeout: Duration, } +/// An admission that ended in a terminal failure, and whether Device enforcement caused it. +struct Refused { + outcome: Outcome, + by_enforcement: bool, +} + +impl From for Refused { + fn from(outcome: Outcome) -> Self { + Self { + outcome, + by_enforcement: false, + } + } +} + +/// Why a Machine's Build capability could not be verified. +#[derive(Debug, thiserror::Error)] +pub(crate) enum CapabilityCheckError { + /// The Machine enforces Device approval and refused the Build. + #[error("{0}")] + RefusedByEnforcement(String), + #[error(transparent)] + Failed(#[from] remote::InputError), +} + async fn open_and_admit( client: &Client, request: Input, @@ -138,11 +173,11 @@ async fn open_and_admit( cancellation: &CancellationToken, progress: &mut impl FnMut(Progress), evidence: &ployz_build::WorkEvidence, -) -> Result { +) -> Result { let (sender, receiver) = mpsc::channel(2); let start = match remote::encode(&request) { Ok(start) => start, - Err(message) => return Err(failed(Stage::Admission, message.to_string())), + Err(message) => return Err(failed(Stage::Admission, message.to_string()).into()), }; sender.send(start).await.expect("request receiver is owned"); let target = MachineTarget::from(&machine_id); @@ -156,22 +191,27 @@ async fn open_and_admit( Ok(Err(error)) => { let refusal = error.to_rpc_error(); // An enforcing Machine's refusal is a sentence of its own, not a failure to reach it. - let message = - if crate::connect::refused_by_enforcement(&refusal, ployz_core::BUILD_CAPABILITY) { - refusal.message - } else { - format!( - "Machine {machine_id} cannot accept the Build: {}", - crate::ui::row(&error) - ) - }; - return Err(failed(Stage::Admission, message)); + if crate::connect::refused_by_enforcement(&refusal, ployz_core::BUILD_CAPABILITY) { + return Err(Refused { + outcome: failed(Stage::Admission, refusal.message), + by_enforcement: true, + }); + } + return Err(failed( + Stage::Admission, + format!( + "Machine {machine_id} cannot accept the Build: {}", + crate::ui::row(&error) + ), + ) + .into()); } Err(_) => { return Err(failed( Stage::Admission, format!("Machine {machine_id} did not answer; source was not uploaded"), - )); + ) + .into()); } }; let waiting_since = tokio::time::Instant::now(); @@ -183,7 +223,7 @@ async fn open_and_admit( () = cancellation.cancelled() => { // No source has left this client. Dropping the stream removes // its connection-scoped waiter, even if admission raced cancellation. - return Err(failed(Stage::Queued, "Build cancelled before upload; execution was not attempted").with_work(evidence.clone())); + return Err(failed(Stage::Queued, "Build cancelled before upload; execution was not attempted").with_work(evidence.clone()).into()); } result = responses.message() => match result { Ok(Some(payload)) => match remote::decode::(&payload) { @@ -195,13 +235,13 @@ async fn open_and_admit( admission_deadline = waiting_since + Duration::from_secs(86410); progress(Progress::Stage(Stage::Queued)); } - Ok(Event::Finished(outcome @ (Outcome::Failed { .. } | Outcome::Unknown { .. }))) => return Err(outcome.with_work(evidence.clone())), - _ => return Err(failed(Stage::Admission, "invalid Build admission response; source was not uploaded").with_work(evidence.clone())), + Ok(Event::Finished(outcome @ (Outcome::Failed { .. } | Outcome::Unknown { .. }))) => return Err(outcome.with_work(evidence.clone()).into()), + _ => return Err(failed(Stage::Admission, "invalid Build admission response; source was not uploaded").with_work(evidence.clone()).into()), }, - Ok(None) => return Err(failed(Stage::Admission, "Build stream ended before admission; source was not uploaded").with_work(evidence.clone())), - Err(status) => return Err(failed(Stage::Admission, format!("{}; source was not uploaded", disconnected(status))).with_work(evidence.clone())), + Ok(None) => return Err(failed(Stage::Admission, "Build stream ended before admission; source was not uploaded").with_work(evidence.clone()).into()), + Err(status) => return Err(failed(Stage::Admission, format!("{}; source was not uploaded", disconnected(status))).with_work(evidence.clone()).into()), }, - () = tokio::time::sleep_until(admission_deadline) => return Err(failed(Stage::Queued, "Build admission deadline expired; source was not uploaded").with_work(evidence.clone())), + () = tokio::time::sleep_until(admission_deadline) => return Err(failed(Stage::Queued, "Build admission deadline expired; source was not uploaded").with_work(evidence.clone()).into()), } }; Ok(Admitted { @@ -253,7 +293,7 @@ async fn execute_attempt( active_timeout, } = match admitted { Ok(admitted) => admitted, - Err(outcome) => return Completion::Report(outcome), + Err(Refused { outcome, .. }) => return Completion::Report(outcome), }; let mut retained = None; let outcome = async { @@ -483,6 +523,31 @@ mod tests { )); } + #[tokio::test] + async fn a_build_refused_by_enforcement_is_told_apart_by_its_capability() { + let (client, _server) = crate::connect::test_support::rpc_bidi_client(|_| async { + Err::>>>, _>( + tonic::Status::with_details( + tonic::Code::PermissionDenied, + ployz_core::BUILD_REFUSED_BY_ENFORCEMENT, + serde_json::json!({ "capability": ployz_core::BUILD_CAPABILITY, "cause": [] }) + .to_string() + .into(), + ), + ) + }) + .await; + let error = client + .check_build_capabilities(MachineId::random(), &[], &CancellationToken::new()) + .await + .unwrap_err(); + assert!( + matches!(&error, CapabilityCheckError::RefusedByEnforcement(message) + if message == ployz_core::BUILD_REFUSED_BY_ENFORCEMENT), + "{error:?}" + ); + } + #[tokio::test] async fn a_dropped_build_stream_names_its_transport_cause() { let (client, _server) = crate::connect::test_support::rpc_bidi_client(|_| async { @@ -493,7 +558,10 @@ mod tests { ]))) }) .await; - let Err(Outcome::Failed { stage, message, .. }) = open_and_admit( + let Err(Refused { + outcome: Outcome::Failed { stage, message, .. }, + .. + }) = open_and_admit( &client, Input::Check(Vec::new()), MachineId::random(), diff --git a/core/crates/ployz/src/sdk/prepare.rs b/core/crates/ployz/src/sdk/prepare.rs index 976732bca5..3f5d5f5dfc 100644 --- a/core/crates/ployz/src/sdk/prepare.rs +++ b/core/crates/ployz/src/sdk/prepare.rs @@ -1,4 +1,5 @@ //! Cloud preparation: builder eligibility, Builds, planning, and image delivery. +use crate::build::CapabilityCheckError; use crate::connect::ConnectError; use ployz_core::{ DescribeContractRequest, MachineId, MachineName, MachineObservation, MachineTarget, RpcError, @@ -178,7 +179,9 @@ pub async fn select_build_machine( crate::ui::row(&error) ), // A refusal by device enforcement is no failed check: it says what it is. - if error.to_string() == ployz_core::BUILD_REFUSED_BY_ENFORCEMENT { + if matches!(error, CapabilityCheckError::RefusedByEnforcement(_)) + || error.to_string() == ployz_core::BUILD_REFUSED_BY_ENFORCEMENT + { ployz_core::BUILD_REFUSED_BY_ENFORCEMENT } else { "its build capability check failed" From 9c8fc394d6dc79ad5bb84d8a2f9a8c038ab37305 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 13:52:44 +0000 Subject: [PATCH 3/7] Share one management dial, its close codes and the Management Client label kind --- .../src/domain/management_capability.rs | 17 ++++++ core/crates/ployz-core/src/value.rs | 58 ++++++++++++++++++ core/crates/ployz/src/cluster.rs | 10 ++- core/crates/ployz/src/connect/build_grant.rs | 17 ++---- core/crates/ployz/src/connect/management.rs | 61 +++++++++++-------- core/crates/ployz/src/handlers/cloud.rs | 3 +- .../ployz/src/handlers/server/remove.rs | 2 +- .../ployzd/src/machine/local_machine.rs | 2 +- core/crates/ployzd/src/management/mod.rs | 8 +-- 9 files changed, 132 insertions(+), 46 deletions(-) diff --git a/core/crates/ployz-core/src/domain/management_capability.rs b/core/crates/ployz-core/src/domain/management_capability.rs index f72946e171..39f4246e03 100644 --- a/core/crates/ployz-core/src/domain/management_capability.rs +++ b/core/crates/ployz-core/src/domain/management_capability.rs @@ -31,6 +31,23 @@ pub const TUNNEL_OPEN: u8 = 0; /// reason says which. pub const TUNNEL_FAILED: u32 = 0x53; +/// QUIC application close code a Machine sends a key it does not admit: never admitted, or +/// rotated away. +pub const CLIENT_REFUSED_CLOSE: u32 = 0x50; + +/// QUIC application close code a Machine sends a live connection whose key no slot holds +/// any more. +pub const REVOKED_CLOSE: u32 = 0x51; + +/// QUIC application close code a Machine sends to confirm, authenticated, that the dialing +/// key was cleared: a Cleared tombstone holds it. +pub const CLIENT_CLEARED_CLOSE: u32 = 0x52; + +/// Whether a management-side endpoint sends with UDP segmentation offload. It does not: a +/// NIC without UDP checksum offload refuses a segmented send, and the socket only learns +/// that by dropping one, which stalls the first large send on a fresh path (#3567). +pub const SEGMENTATION_OFFLOAD: bool = false; + /// The [`TUNNEL_FAILED`] close reason for a client below `Full` scope, matched whole. A /// v0.3.4 CLI prints it as it is, and knows no other close code, so it names the way on. pub const TUNNEL_NEEDS_FULL: &str = "tunnels need Full access: run `ployz device request` (update ployz first if it has no `device` command)"; diff --git a/core/crates/ployz-core/src/value.rs b/core/crates/ployz-core/src/value.rs index 1426aea901..7276fdf997 100644 --- a/core/crates/ployz-core/src/value.rs +++ b/core/crates/ployz-core/src/value.rs @@ -521,6 +521,44 @@ validated_string_newtype!( && bytes.all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'-') } ); +/// The namespace a [`ManagementClientLabel`] names, read from its text. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ManagementClientKind { + Cloud, + Cli, + Device, + Other, +} + +impl ManagementClientLabel { + const CLOUD: &'static str = "cloud"; + const CLI_PREFIX: &'static str = "cli-"; + + /// The label Cloud's slot carries. + #[must_use] + pub fn cloud() -> Self { + Self::parse(Self::CLOUD).expect("`cloud` is a valid Management Client label") + } + + #[must_use] + pub fn is_cloud(&self) -> bool { + self.as_str() == Self::CLOUD + } + + #[must_use] + pub fn kind(&self) -> ManagementClientKind { + if self.is_cloud() { + ManagementClientKind::Cloud + } else if self.as_str().starts_with(Self::CLI_PREFIX) { + ManagementClientKind::Cli + } else if crate::DeviceCode::reserves(self) { + ManagementClientKind::Device + } else { + ManagementClientKind::Other + } + } +} + validated_string_newtype!( DockerVolumeName, "Docker Volume name", @@ -1278,6 +1316,26 @@ mod tests { use super::*; + #[test] + fn a_management_client_label_names_its_kind() { + for (label, kind) in [ + ("cloud", ManagementClientKind::Cloud), + ( + "cli-0123456789abcdef0123456789ab", + ManagementClientKind::Cli, + ), + ("device-abc", ManagementClientKind::Device), + ("laptop", ManagementClientKind::Other), + ("cloud-2", ManagementClientKind::Other), + ("cli", ManagementClientKind::Other), + ] { + let label = ManagementClientLabel::parse(label).unwrap(); + assert_eq!(label.kind(), kind, "{label}"); + assert_eq!(label.is_cloud(), kind == ManagementClientKind::Cloud); + } + assert_eq!(ManagementClientLabel::cloud().as_str(), "cloud"); + } + #[test] fn a_hostname_normalises_to_a_machine_name() { let long = format!("{}-tail", "a".repeat(66)); diff --git a/core/crates/ployz/src/cluster.rs b/core/crates/ployz/src/cluster.rs index e63324b10c..0fa9529f9c 100644 --- a/core/crates/ployz/src/cluster.rs +++ b/core/crates/ployz/src/cluster.rs @@ -2008,11 +2008,17 @@ pub(crate) async fn refuse_last_managed( )) })? .management_clients; - let cloud = holders.iter().any(|label| label.as_str() == "cloud"); + let cloud = holders + .iter() + .any(ployz_core::ManagementClientLabel::is_cloud); let names: Vec = holders .iter() .filter(|label| { - !cloud || (label.as_str() != "cloud" && !label.as_str().starts_with("cli-")) + !cloud + || !matches!( + label.kind(), + ployz_core::ManagementClientKind::Cloud | ployz_core::ManagementClientKind::Cli + ) }) .map(|label| format!("`{label}`")) .collect(); diff --git a/core/crates/ployz/src/connect/build_grant.rs b/core/crates/ployz/src/connect/build_grant.rs index 297467f8b1..8b85422e7f 100644 --- a/core/crates/ployz/src/connect/build_grant.rs +++ b/core/crates/ployz/src/connect/build_grant.rs @@ -4,8 +4,8 @@ use std::{io, net::SocketAddr}; use iroh::{ - Endpoint, EndpointAddr, PublicKey, RelayMode, SecretKey, - endpoint::{Connection, ConnectionError, VarInt, presets}, + Endpoint, PublicKey, SecretKey, + endpoint::{Connection, ConnectionError, VarInt}, }; use ployz_core::{ BUILD_GRANT_ALPN, BUILD_GRANT_ENDED, BUILD_GRANT_REFUSED, BuildGrant, OpenedSealed, @@ -13,7 +13,7 @@ use ployz_core::{ }; use tokio::net::TcpListener; -use super::ManagementRelay; +use super::{ManagementRelay, management}; /// A loopback registry address forwarding each TCP connection to the grant transport. /// Dropping it stops forwarding and closes the connection. @@ -121,18 +121,11 @@ pub async fn open_grant_registry( ) -> io::Result { let machine = PublicKey::from_bytes(grant.machine().as_bytes()) .map_err(|_| io::Error::other("the Build Grant names an invalid Machine identity"))?; - let endpoint = Endpoint::builder(presets::Minimal) - .secret_key(SecretKey::from_bytes(grant.secret())) - .relay_mode(RelayMode::custom([relay.url.clone()])) - .ca_tls_config(relay.tls.clone()) + let endpoint = management::endpoint(SecretKey::from_bytes(grant.secret()), relay) .bind() .await .map_err(io::Error::other)?; - let connection = endpoint - .connect( - EndpointAddr::new(machine).with_relay_url(relay.url.clone()), - BUILD_GRANT_ALPN, - ) + let connection = management::dial(&endpoint, machine, relay, BUILD_GRANT_ALPN) .await .map_err(io::Error::other)?; let listener = TcpListener::bind((std::net::Ipv4Addr::LOCALHOST, 0)).await?; diff --git a/core/crates/ployz/src/connect/management.rs b/core/crates/ployz/src/connect/management.rs index 6875e2d6cf..07877969c9 100644 --- a/core/crates/ployz/src/connect/management.rs +++ b/core/crates/ployz/src/connect/management.rs @@ -11,9 +11,10 @@ use iroh::{ tls::CaTlsConfig, }; use ployz_core::{ - DEFAULT_RELAY_URL, DescribeContractRequest, MANAGEMENT_ALPN, MANAGEMENT_PROOF_LABEL, - MachineRpcClient, ManagementCapability, ManagementProof, TUNNEL_ALPN, TUNNEL_FAILED, - TUNNEL_NEEDS_FULL, TUNNEL_OPEN, op, + CLIENT_CLEARED_CLOSE, CLIENT_REFUSED_CLOSE, DEFAULT_RELAY_URL, DescribeContractRequest, + MANAGEMENT_ALPN, MANAGEMENT_PROOF_LABEL, MachineRpcClient, ManagementCapability, + ManagementProof, SEGMENTATION_OFFLOAD, TUNNEL_ALPN, TUNNEL_FAILED, TUNNEL_NEEDS_FULL, + TUNNEL_OPEN, op, }; use std::{ io, @@ -27,8 +28,8 @@ use tonic::transport::Channel; /// Application close codes the daemon refuses an unadmitted key with: never admitted or /// rotated away, and confirmed cleared. -const CLIENT_REFUSED: VarInt = VarInt::from_u32(0x50); -const CLIENT_CLEARED: VarInt = VarInt::from_u32(0x52); +const CLIENT_REFUSED: VarInt = VarInt::from_u32(CLIENT_REFUSED_CLOSE); +const CLIENT_CLEARED: VarInt = VarInt::from_u32(CLIENT_CLEARED_CLOSE); const TUNNEL_CLOSE_GRACE: Duration = Duration::from_secs(2); /// The relay the management transport dials through. Production uses the compiled @@ -100,11 +101,7 @@ impl Drop for ManagementEndpoint { async fn management_endpoint( relay: &ManagementRelay, ) -> Result, ConnectError> { - let mut builder = IrohEndpoint::builder(presets::Minimal) - .secret_key(SecretKey::generate()) - .relay_mode(RelayMode::custom([relay.url.clone()])) - .ca_tls_config(relay.tls.clone()) - .transport_config(transport()); + let mut builder = endpoint(SecretKey::generate(), relay); if relay.relay_only { builder = builder.clear_ip_transports(); } @@ -118,12 +115,36 @@ async fn management_endpoint( })) } -/// Sends without segmentation offload, as the daemon does (#3567): a NIC without UDP -/// checksum offload refuses a segmented send, and the socket only learns that by -/// dropping one, which on this fresh endpoint is the command's first large request. +/// The builder every management-side dial binds from: the relay and its trust roots, and +/// the same send setting the daemon has (#3567). +pub(super) fn endpoint(secret: SecretKey, relay: &ManagementRelay) -> iroh::endpoint::Builder { + IrohEndpoint::builder(presets::Minimal) + .secret_key(secret) + .relay_mode(RelayMode::custom([relay.url.clone()])) + .ca_tls_config(relay.tls.clone()) + .transport_config(transport()) +} + +/// Dial `machine` on `alpn` with only the relay as a hint. +pub(super) async fn dial( + endpoint: &IrohEndpoint, + machine: PublicKey, + relay: &ManagementRelay, + alpn: &[u8], +) -> Result { + endpoint + .connect( + EndpointAddr::new(machine).with_relay_url(relay.url.clone()), + alpn, + ) + .await +} + +/// Sends without segmentation offload, as the daemon does: on this fresh endpoint a +/// segmented send that a NIC refuses is the command's first large request. fn transport() -> QuicTransportConfig { QuicTransportConfig::builder() - .enable_segmentation_offload(false) + .enable_segmentation_offload(SEGMENTATION_OFFLOAD) .build() } @@ -192,11 +213,8 @@ pub(super) async fn connect_management( let endpoint = management_endpoint(relay).await; stage.finish(&endpoint); let endpoint = endpoint?; - let address = EndpointAddr::new(machine).with_relay_url(relay.url.clone()); let mut stage = StageGuard::begin(Stage::IrohConnect); - let connection = endpoint - .endpoint - .connect(address, MANAGEMENT_ALPN) + let connection = dial(&endpoint.endpoint, machine, relay, MANAGEMENT_ALPN) .await .map_err(|error| dial_error("management dial", &error)); stage.finish(&connection); @@ -272,12 +290,7 @@ pub(super) async fn dial_tunnel( let machine = PublicKey::from_bytes(capability.machine().as_bytes()) .map_err(|_| ConnectionError::ManagementCapability)?; let endpoint = management_endpoint(relay).await?; - let connection = endpoint - .endpoint - .connect( - EndpointAddr::new(machine).with_relay_url(relay.url.clone()), - TUNNEL_ALPN, - ) + let connection = dial(&endpoint.endpoint, machine, relay, TUNNEL_ALPN) .await .map_err(|error| dial_error("tunnel dial", &error))?; let session = Arc::new(ManagementConnection { diff --git a/core/crates/ployz/src/handlers/cloud.rs b/core/crates/ployz/src/handlers/cloud.rs index 2bd15aa4f7..f3305f3654 100644 --- a/core/crates/ployz/src/handlers/cloud.rs +++ b/core/crates/ployz/src/handlers/cloud.rs @@ -928,8 +928,7 @@ async fn set_cloud_management_client( let response = client .call_repeatable::( SetManagementClientRequest::Set { - label: ManagementClientLabel::parse("cloud") - .expect("`cloud` is a valid Management Client label"), + label: ManagementClientLabel::cloud(), }, None, ) diff --git a/core/crates/ployz/src/handlers/server/remove.rs b/core/crates/ployz/src/handlers/server/remove.rs index 1a49434348..c0b959aac0 100644 --- a/core/crates/ployz/src/handlers/server/remove.rs +++ b/core/crates/ployz/src/handlers/server/remove.rs @@ -608,7 +608,7 @@ async fn cloud_manages( Ok(details .management_clients .iter() - .any(|label| label.as_str() == "cloud")) + .any(ployz_core::ManagementClientLabel::is_cloud)) } /// The warning that the Server runs Services a drain could move off it, with the drain diff --git a/core/crates/ployzd/src/machine/local_machine.rs b/core/crates/ployzd/src/machine/local_machine.rs index 037cadf9fa..66be51f82d 100644 --- a/core/crates/ployzd/src/machine/local_machine.rs +++ b/core/crates/ployzd/src/machine/local_machine.rs @@ -421,7 +421,7 @@ impl LocalMachine { return Ok(()); }; let label = record.accepted_label(&remote); - if label.is_some_and(|label| label.as_str() == "cloud") { + if label.is_some_and(ManagementClientLabel::is_cloud) { return Ok(()); } tracing::warn!( diff --git a/core/crates/ployzd/src/management/mod.rs b/core/crates/ployzd/src/management/mod.rs index c78956000d..7e860c715f 100644 --- a/core/crates/ployzd/src/management/mod.rs +++ b/core/crates/ployzd/src/management/mod.rs @@ -46,11 +46,11 @@ pub use build_grant::{BuildGrants, Mint}; pub use forgotten::Roster; /// Application close code sent when the remote key is not admitted and no tombstone holds it. -pub const CLIENT_REFUSED: VarInt = VarInt::from_u32(0x50); +pub const CLIENT_REFUSED: VarInt = VarInt::from_u32(ployz_core::CLIENT_REFUSED_CLOSE); /// Application close code sent to a live connection whose key no slot holds any more. -pub const REVOKED: VarInt = VarInt::from_u32(0x51); +pub const REVOKED: VarInt = VarInt::from_u32(ployz_core::REVOKED_CLOSE); /// Authenticated confirmation that the dialing key was cleared: a Cleared tombstone holds it. -pub const CLIENT_CLEARED: VarInt = VarInt::from_u32(0x52); +pub const CLIENT_CLEARED: VarInt = VarInt::from_u32(ployz_core::CLIENT_CLEARED_CLOSE); const MAX_CONCURRENT_HANDSHAKES: usize = 64; const IDLE_TIMEOUT: Duration = Duration::from_secs(60); @@ -158,7 +158,7 @@ fn transport() -> QuicTransportConfig { .max_idle_timeout(Some( IdleTimeout::try_from(IDLE_TIMEOUT).expect("60s is within the idle timeout bounds"), )) - .enable_segmentation_offload(false) + .enable_segmentation_offload(ployz_core::SEGMENTATION_OFFLOAD) .build() } From 50e11b5b27a74b0fe9e373860cbed6ce280de19e Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 14:02:42 +0000 Subject: [PATCH 4/7] Answer a transport failure's questions from typed origin and routing, in one facts table --- core/crates/ployz/src/connect.rs | 265 +++++++++++++++---------- core/crates/ployz/src/connect/tests.rs | 212 +++++++++++++++++++- 2 files changed, 371 insertions(+), 106 deletions(-) diff --git a/core/crates/ployz/src/connect.rs b/core/crates/ployz/src/connect.rs index 18ff9e2c40..da89fd2c78 100644 --- a/core/crates/ployz/src/connect.rs +++ b/core/crates/ployz/src/connect.rs @@ -264,7 +264,7 @@ pub(crate) fn on_closed_session(error: &RpcError) -> bool { #[cfg(test)] pub(crate) fn closed_session_error() -> RpcError { TransportError { - session_closed: true, + origin: Origin::SessionClosed, ..TransportError::from(tonic::Status::unavailable("transport error")) } .to_rpc_error() @@ -1128,7 +1128,7 @@ fn naming_local_refusal(connection: &Connection, error: ConnectError) -> Connect let ConnectError::Rpc(transport) = error else { return error; }; - if !transport.local { + if !transport.is_local() { return ConnectError::Rpc(transport); } match path.parent().and_then(ployz_core::startup_refusal) { @@ -1395,6 +1395,20 @@ impl From for ConnectError { } } +/// What a [`ConnectError`] says about a call: whether to repeat it, and what is known of +/// the target. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) struct Facts { + pub(crate) retry: Retry, + pub(crate) unreachable: bool, + pub(crate) unreached: bool, + pub(crate) routed_by_entry: bool, + pub(crate) target_unanswered: bool, + pub(crate) target_replied: bool, + pub(crate) unreplied: bool, + pub(crate) target_unreached: bool, +} + impl ConnectError { fn from_ssh_spawn(error: io::Error) -> Self { if error.kind() == io::ErrorKind::NotFound { @@ -1404,48 +1418,93 @@ impl ConnectError { } } - pub(crate) fn retry(&self) -> Retry { + /// Every answer a [`ConnectError`] gives about the call, in one table. + pub(crate) fn facts(&self) -> Facts { + let none = Facts { + retry: Retry::Never, + unreachable: false, + unreached: false, + routed_by_entry: false, + target_unanswered: false, + target_replied: false, + unreplied: false, + target_unreached: false, + }; match self { - Self::Attempt(_) | Self::Exhausted(_) | Self::Dial(_) | Self::Join(_) => Retry::Now, - Self::Io(error) if transient_io(error) => Retry::Now, - Self::SshProbe { - failure: SshFailure::Network, - .. - } => Retry::Now, - Self::Rpc(error) => match rpc_error_code(error.code).retry() { - // A restarting daemon drops the connection without answering. - Retry::Never if error.unanswered => Retry::AfterRestart, - retry @ (Retry::Now | Retry::AfterRestart | Retry::Never) => retry, + Self::Attempt(_) | Self::Exhausted(_) | Self::Dial(_) => Facts { + retry: Retry::Now, + unreachable: true, + unreached: true, + ..none + }, + Self::Join(_) => Facts { + retry: Retry::Now, + ..none + }, + Self::Io(error) => Facts { + retry: if transient_io(error) { + Retry::Now + } else { + Retry::Never + }, + unreachable: true, + unreached: true, + ..none }, + Self::SshProbe { failure, .. } => Facts { + retry: if *failure == SshFailure::Network { + Retry::Now + } else { + Retry::Never + }, + unreached: true, + ..none + }, + Self::Rpc(error) => error.facts(), // A socket-activated daemon accepts before it serves. - Self::EntryNotReady => Retry::AfterRestart, - Self::AllFailed { retry, .. } => *retry, - Self::Io(_) - | Self::SshProbe { .. } - | Self::Remote(_) - | Self::IdentityMismatch { .. } + Self::EntryNotReady => Facts { + retry: Retry::AfterRestart, + unreachable: true, + unreached: true, + ..none + }, + Self::AllFailed { retry, reached, .. } => Facts { + retry: *retry, + unreachable: true, + unreached: !reached, + ..none + }, + Self::SshClientMissing + | Self::ProxyUnsupported(_) + | Self::UnsupportedNetwork(_) + | Self::Config(_) + | Self::Connection(_) + | Self::Context(_) + | Self::Path { .. } => Facts { + unreached: true, + ..none + }, + Self::IdentityMismatch { .. } | Self::ClusterMismatch { .. } | Self::ClusterUnverified { .. } | Self::PredatesSealing { .. } + | Self::MissingMachineDetails | Self::ClientRefused | Self::OlderMachine | Self::ClientCleared | Self::TunnelNeedsFull - | Self::MissingMachineDetails - | Self::SshClientMissing | Self::Routing(_) - | Self::ProxyUnsupported(_) - | Self::UnsupportedNetwork(_) - | Self::Config(_) - | Self::Connection(_) - | Self::Context(_) - | Self::Path { .. } | Self::Codec(_) + | Self::Remote(_) | Self::Framing(_) - | Self::Value(_) => Retry::Never, + | Self::Value(_) => none, } } + pub(crate) fn retry(&self) -> Retry { + self.facts().retry + } + /// Repeatable reads retry until the daemon answers; an answer is final. /// They must not spend a minute retrying missing credentials or keys. pub(crate) fn is_setup_retryable(&self) -> bool { @@ -1476,80 +1535,39 @@ impl ConnectError { } pub(crate) fn is_unreachable(&self) -> bool { - matches!( - self, - Self::Attempt(_) - | Self::Exhausted(_) - | Self::EntryNotReady - | Self::Io(_) - | Self::Dial(_) - | Self::AllFailed { .. } - ) || matches!(self, Self::Rpc(error) if error.is_unavailable()) + self.facts().unreachable } /// No daemon answered: selection, dialing or the transport failed first. A daemon's /// own refusal, identity or protocol is an answer, and so is anything unclassified. pub(crate) fn unreached(&self) -> bool { - match self { - Self::Attempt(_) - | Self::Exhausted(_) - | Self::EntryNotReady - | Self::Io(_) - | Self::Dial(_) - | Self::SshClientMissing - | Self::SshProbe { .. } - | Self::ProxyUnsupported(_) - | Self::UnsupportedNetwork(_) - | Self::Config(_) - | Self::Connection(_) - | Self::Context(_) - | Self::Path { .. } => true, - Self::Rpc(error) => error.local || error.budget_expired, - Self::AllFailed { reached, .. } => !reached, - Self::IdentityMismatch { .. } - | Self::ClusterMismatch { .. } - | Self::ClusterUnverified { .. } - | Self::PredatesSealing { .. } - | Self::MissingMachineDetails - | Self::ClientRefused - | Self::OlderMachine - | Self::ClientCleared - | Self::TunnelNeedsFull - | Self::Routing(_) - | Self::Join(_) - | Self::Codec(_) - | Self::Remote(_) - | Self::Framing(_) - | Self::Value(_) => false, - } + self.facts().unreached } /// The entry's router made this status for a target that didn't answer, and it came back /// over a working channel: the entry itself answered. pub(crate) fn routed_by_entry(&self) -> bool { - matches!(self, Self::Rpc(error) - if error.routed_unanswered && !error.local && !error.session_closed) + self.facts().routed_by_entry } /// The read's whole budget ran out without a reply, or the entry's router could not /// get one from the target. A status a daemon sent, or a failure reaching the entry, /// says nothing about whether the target lives. pub(crate) fn target_unanswered(&self) -> bool { - matches!(self, Self::Rpc(error) if error.budget_expired || error.routed_unanswered) + self.facts().target_unanswered } /// A daemon sent this status itself, so the target was not silent. A call on /// a closed session never left this client, so nothing replied to it. pub(crate) fn target_replied(&self) -> bool { - matches!(self, Self::Rpc(error) if !error.session_closed) && !self.unreplied() + self.facts().target_replied } /// The call went out and no status came back: the client's connection dropped, its /// deadline elapsed, or the entry's router heard nothing from the target. Whether the /// target acted on it is unknown. pub(crate) fn unreplied(&self) -> bool { - matches!(self, Self::Rpc(error) - if error.unanswered || error.budget_expired || error.routed_unanswered) + self.facts().unreplied } /// The entry's router said its path to the target drops large packets. @@ -1562,7 +1580,7 @@ impl ConnectError { /// The entry's router could not dial the target, so the target never got the call. /// An entry Machine too old to say so leaves this false. pub(crate) fn target_unreached(&self) -> bool { - matches!(self, Self::Rpc(error) if error.routed_unreached) + self.facts().target_unreached } } @@ -1598,6 +1616,30 @@ pub(crate) fn keep_lost_call(lost: Option, error: ConnectError) -> } } +/// Where a [`TransportError`] was made. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Origin { + /// A status that crossed the wire. + Remote, + /// The client's connection or stream failed: no status crossed the wire. + Local, + /// The channel's one stream closed: resending on it can never succeed, so + /// [`TransportError::to_rpc_error`] marks it for [`on_closed_session`]. It never left + /// this client, so it is a local failure too. + SessionClosed, +} + +/// What the entry's router marked a status with. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Routed { + /// The target did not answer. + Unanswered, + /// The router's dial to the target failed, so the target never got the call. It rides + /// on an unanswered status, and is never read off a local failure, whose metadata may + /// be headers that came back before the stream broke. + Unreached, +} + /// A gRPC transport failure. Display is the status message, not Status Debug. #[derive(Clone, Debug, Eq, Error, PartialEq)] #[error("{message}")] @@ -1607,19 +1649,11 @@ pub struct TransportError { details: Value, /// Worth retrying as a silence: the client produced this status, or it is Cancelled. unanswered: bool, - /// The client's connection or stream failed: no status crossed the wire. - local: bool, + origin: Origin, + /// What the entry's router marked this status with, if it did. + routed: Option, /// The client's own deadline for the whole call elapsed. budget_expired: bool, - /// The entry's router marked this status: the target did not answer it. - routed_unanswered: bool, - /// The entry's router marked this status: its dial to the target failed, so the - /// target never got the call. Never set on a local failure, whose metadata may be - /// headers that came back before the stream broke. - routed_unreached: bool, - /// The channel's one stream closed: resending on it can never succeed, so - /// [`TransportError::to_rpc_error`] marks it for [`on_closed_session`]. - session_closed: bool, /// The status carried details, which ployzd attaches and tonic or a proxy never does. detailed: bool, /// Why the client's connection or stream failed, below tonic's own words. @@ -1632,11 +1666,9 @@ impl TransportError { /// ployzd attaches. A status tonic or a proxy made up carries no details. pub(crate) fn answered(&self) -> bool { !(self.unanswered - || self.local + || self.origin != Origin::Remote || self.budget_expired - || self.routed_unanswered - || self.routed_unreached - || self.session_closed + || self.routed.is_some() || self.message.is_empty() || !self.detailed) } @@ -1645,7 +1677,29 @@ impl TransportError { /// call was cancelled, its session closed, or the entry marked the target silent or unreached. /// A status tonic or the daemon made, such as a decode failure, is not one. pub(crate) fn connection_lost(&self) -> bool { - self.unanswered || self.session_closed || self.routed_unanswered || self.routed_unreached + self.unanswered || self.origin == Origin::SessionClosed || self.routed.is_some() + } + + fn is_local(&self) -> bool { + self.origin != Origin::Remote + } + + fn facts(&self) -> Facts { + let unreplied = self.unanswered || self.budget_expired || self.routed.is_some(); + Facts { + retry: match rpc_error_code(self.code).retry() { + // A restarting daemon drops the connection without answering. + Retry::Never if self.unanswered => Retry::AfterRestart, + retry @ (Retry::Now | Retry::AfterRestart | Retry::Never) => retry, + }, + unreachable: self.is_unavailable(), + unreached: self.is_local() || self.budget_expired, + routed_by_entry: self.routed.is_some() && self.origin == Origin::Remote, + target_unanswered: self.budget_expired || self.routed.is_some(), + target_replied: self.origin != Origin::SessionClosed && !unreplied, + unreplied, + target_unreached: self.routed == Some(Routed::Unreached), + } } /// `error`, which ended the client before the stream opened, as [`rpc_error`] gives it. @@ -1727,7 +1781,7 @@ impl TransportError { #[must_use] pub fn to_rpc_error(&self) -> RpcError { RpcError::new(rpc_error_code(self.code), self.message.clone()) - .with_details(if self.session_closed { + .with_details(if self.origin == Origin::SessionClosed { json!({ "reason": SESSION_CLOSED_REASON }) } else { self.details.clone() @@ -1762,18 +1816,25 @@ impl From for TransportError { } None => sent.clone().and_then(crate::failure::JoinedChain::of), }; + let marked = |key| status.metadata().contains_key(key); + let routed = if !local && marked(ployz_core::rpc::TARGET_UNREACHED_KEY) { + Some(Routed::Unreached) + } else if marked(ployz_core::rpc::TARGET_UNANSWERED_KEY) { + Some(Routed::Unanswered) + } else { + None + }; Self { unanswered, - local, + origin: if session_closed { + Origin::SessionClosed + } else if local { + Origin::Local + } else { + Origin::Remote + }, budget_expired: false, - routed_unanswered: status - .metadata() - .contains_key(ployz_core::rpc::TARGET_UNANSWERED_KEY), - routed_unreached: !local - && status - .metadata() - .contains_key(ployz_core::rpc::TARGET_UNREACHED_KEY), - session_closed, + routed, detailed: !status.details().is_empty(), cause, code: status.code(), diff --git a/core/crates/ployz/src/connect/tests.rs b/core/crates/ployz/src/connect/tests.rs index 78c893428a..5f4577c64d 100644 --- a/core/crates/ployz/src/connect/tests.rs +++ b/core/crates/ployz/src/connect/tests.rs @@ -1301,7 +1301,7 @@ fn a_call_that_lost_its_answer_outlasts_a_retry_that_never_reached_the_target() fn a_closed_session_does_not_outlast_a_later_silence() { let closed = || { ConnectError::Rpc(TransportError { - session_closed: true, + origin: Origin::SessionClosed, ..TransportError::from(tonic::Status::unavailable("transport error")) }) }; @@ -1393,14 +1393,14 @@ async fn a_call_after_the_stream_closed_is_marked_and_stays_unavailable() { request_headers(&mut daemon).await; drop(daemon); let lost = TransportError::from(in_flight.await.unwrap().unwrap_err()); - assert!(!lost.session_closed, "{lost:?}"); + assert!(lost.origin != Origin::SessionClosed, "{lost:?}"); assert!( lost.unanswered, "the first call may have reached the daemon" ); let closed = TransportError::from(rpc.describe_contract(describe()).await.unwrap_err()); - assert!(closed.session_closed, "{closed:?}"); + assert!(closed.origin == Origin::SessionClosed, "{closed:?}"); assert!(!closed.unanswered, "a call on a closed session never left"); let connect = ConnectError::Rpc(closed); assert_eq!(connect.retry(), Retry::Now, "a redial opens a new stream"); @@ -1422,7 +1422,7 @@ fn a_broken_pipe_is_not_a_closed_session() { "stream closed because of a broken pipe", ))); let error = TransportError::from(status); - assert!(!error.session_closed, "{error:?}"); + assert!(error.origin != Origin::SessionClosed, "{error:?}"); assert!(error.unanswered); assert!(!on_closed_session(&error.to_rpc_error())); } @@ -2300,3 +2300,207 @@ async fn a_confirmation_telling_another_key_fails_without_inspect() { matches!(*last, ConnectError::ClusterMismatch { machine_id } if Some(&machine_id) == ids.first()) ); } + +/// The six flags a `TransportError` held before it carried `Origin` and `Routed`, and the +/// predicates that read them. They are the oracle `ConnectError::facts` is held to. +#[derive(Clone, Copy, Debug)] +struct OldFlags { + unanswered: bool, + local: bool, + budget_expired: bool, + routed_unanswered: bool, + routed_unreached: bool, + session_closed: bool, +} + +impl OldFlags { + /// The combinations a status can decode to: a closed session is a local failure, and + /// an unreached mark rides on an unanswered one and is never read off a local failure. + fn producible(self) -> bool { + (!self.session_closed || self.local) + && (!self.routed_unreached || (self.routed_unanswered && !self.local)) + } + + fn transport(self, code: tonic::Code) -> TransportError { + TransportError { + unanswered: self.unanswered, + origin: if self.session_closed { + Origin::SessionClosed + } else if self.local { + Origin::Local + } else { + Origin::Remote + }, + routed: if self.routed_unreached { + Some(Routed::Unreached) + } else if self.routed_unanswered { + Some(Routed::Unanswered) + } else { + None + }, + budget_expired: self.budget_expired, + ..TransportError::from(tonic::Status::new(code, "call failed")) + } + } + + fn unreplied(self) -> bool { + self.unanswered || self.budget_expired || self.routed_unanswered + } + + fn facts(self, code: tonic::Code) -> Facts { + Facts { + retry: match rpc_error_code(code).retry() { + ployz_core::Retry::Never if self.unanswered => ployz_core::Retry::AfterRestart, + retry @ (ployz_core::Retry::Now + | ployz_core::Retry::AfterRestart + | ployz_core::Retry::Never) => retry, + }, + unreachable: code == tonic::Code::Unavailable, + unreached: self.local || self.budget_expired, + routed_by_entry: self.routed_unanswered && !self.local && !self.session_closed, + target_unanswered: self.budget_expired || self.routed_unanswered, + target_replied: !self.session_closed && !self.unreplied(), + unreplied: self.unreplied(), + target_unreached: self.routed_unreached, + } + } +} + +#[test] +fn a_transport_errors_facts_match_the_flags_they_replaced() { + let mut checked = 0; + for bits in 0..64_u8 { + let flag = |bit: u8| bits & (1 << bit) != 0; + let flags = OldFlags { + unanswered: flag(0), + local: flag(1), + budget_expired: flag(2), + routed_unanswered: flag(3), + routed_unreached: flag(4), + session_closed: flag(5), + }; + if !flags.producible() { + continue; + } + for code in [ + tonic::Code::Unavailable, + tonic::Code::Cancelled, + tonic::Code::InvalidArgument, + tonic::Code::Unimplemented, + tonic::Code::DeadlineExceeded, + ] { + let error = ConnectError::Rpc(flags.transport(code)); + assert_eq!(error.facts(), flags.facts(code), "{flags:?} {code:?}"); + checked += 1; + } + } + assert!(checked > 100, "{checked}"); +} + +#[test] +fn a_status_decodes_to_the_origin_and_routing_it_carried() { + let marked = |keys: &[&'static str]| { + let mut status = tonic::Status::unavailable("target did not answer"); + for key in keys { + status + .metadata_mut() + .insert(*key, tonic::metadata::MetadataValue::from_static("1")); + } + TransportError::from(status) + }; + let unanswered = marked(&[ployz_core::rpc::TARGET_UNANSWERED_KEY]); + assert_eq!( + (unanswered.origin, unanswered.routed), + (Origin::Remote, Some(Routed::Unanswered)) + ); + let unreached = marked(&[ + ployz_core::rpc::TARGET_UNANSWERED_KEY, + ployz_core::rpc::TARGET_UNREACHED_KEY, + ]); + assert_eq!( + (unreached.origin, unreached.routed), + (Origin::Remote, Some(Routed::Unreached)) + ); + assert_eq!(marked(&[]).routed, None); +} + +#[test] +fn every_other_connect_error_keeps_its_facts() { + use ployz_core::Retry::{AfterRestart, Never, Now}; + use std::os::unix::process::ExitStatusExt as _; + let ssh = |failure| ConnectError::SshProbe { + target: "host".into(), + status: std::process::ExitStatus::from_raw(255 << 8), + failure, + detail: String::new(), + }; + let all_failed = |reached| ConnectError::AllFailed { + selection: ConnectionSource::Direct, + attempts: 2, + server: None, + relayed: false, + retry: Now, + reached, + last: None, + }; + for (error, retry, unreachable, unreached) in [ + (ConnectError::Attempt("x".into()), Now, true, true), + (ConnectError::Exhausted("x".into()), Now, true, true), + ( + ConnectError::Io(io::ErrorKind::ConnectionRefused.into()), + Now, + true, + true, + ), + ( + ConnectError::Io(io::ErrorKind::InvalidData.into()), + Never, + true, + true, + ), + (ssh(SshFailure::Network), Now, false, true), + (ssh(SshFailure::Auth), Never, false, true), + (ConnectError::EntryNotReady, AfterRestart, true, true), + (all_failed(false), Now, true, true), + (all_failed(true), Now, true, false), + (ConnectError::SshClientMissing, Never, false, true), + ( + ConnectError::ProxyUnsupported("x".into()), + Never, + false, + true, + ), + ( + ConnectError::UnsupportedNetwork("x".into()), + Never, + false, + true, + ), + (ConnectError::ClientRefused, Never, false, false), + (ConnectError::ClientCleared, Never, false, false), + (ConnectError::OlderMachine, Never, false, false), + (ConnectError::TunnelNeedsFull, Never, false, false), + (ConnectError::MissingMachineDetails, Never, false, false), + ( + ConnectError::Remote(RpcError::internal("x")), + Never, + false, + false, + ), + ] { + assert_eq!( + error.facts(), + Facts { + retry, + unreachable, + unreached, + routed_by_entry: false, + target_unanswered: false, + target_replied: false, + unreplied: false, + target_unreached: false, + }, + "{error:?}" + ); + } +} From 737d0c4cd3d73641da7a31d0a3a98fdac59d2f3e Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 14:16:00 +0000 Subject: [PATCH 5/7] Name the capability check's own failure closure for what it is --- core/crates/ployz/src/build/remote.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/crates/ployz/src/build/remote.rs b/core/crates/ployz/src/build/remote.rs index c1bad8a4fd..b383491165 100644 --- a/core/crates/ployz/src/build/remote.rs +++ b/core/crates/ployz/src/build/remote.rs @@ -59,18 +59,19 @@ impl Client { // Keep the admitted sender alive: closing it would cancel the worker check. let deadline = tokio::time::Instant::now() + admitted.active_timeout + Duration::from_secs(70); - let failed = |message: &str| CapabilityCheckError::from(remote::InputError::from(message)); + let unusable = + |message: &str| CapabilityCheckError::from(remote::InputError::from(message)); tokio::select! { biased; - () = cancellation.cancelled() => Err(failed("Build selection cancelled")), - () = tokio::time::sleep_until(deadline) => Err(failed("Build capability deadline expired")), + () = cancellation.cancelled() => Err(unusable("Build selection cancelled")), + () = tokio::time::sleep_until(deadline) => Err(unusable("Build capability deadline expired")), response = admitted.responses.message() => { let payload = response.map_err(|error| remote::InputError::from(error.to_string()))? - .ok_or_else(|| failed("Build capability response ended early"))?; + .ok_or_else(|| unusable("Build capability response ended early"))?; match remote::decode::(&payload)? { Event::Finished(Outcome::CapabilitiesChecked { machine_id: actual }) if actual == machine_id => Ok(()), Event::Finished(Outcome::Failed { message, .. } | Outcome::Unknown { message, .. }) => Err(remote::InputError::from(message).into()), - Event::Admitted { .. } | Event::Progress(_) | Event::Finished(_) => Err(failed("invalid Build capability response")), + Event::Admitted { .. } | Event::Progress(_) | Event::Finished(_) => Err(unusable("invalid Build capability response")), } } } From fe7eb07ed15d35f90220794b761091b4c3ad7ee7 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 14:22:06 +0000 Subject: [PATCH 6/7] Drop comments that restate the names they sit on --- core/crates/ployz-core/src/rpc.rs | 2 -- core/crates/ployz/src/build/remote.rs | 3 --- core/crates/ployz/src/connect.rs | 1 + core/crates/ployz/src/connect/management.rs | 5 ----- core/crates/ployz/src/connect/tests.rs | 4 ---- 5 files changed, 1 insertion(+), 14 deletions(-) diff --git a/core/crates/ployz-core/src/rpc.rs b/core/crates/ployz-core/src/rpc.rs index 962063584e..5379d62ebb 100644 --- a/core/crates/ployz-core/src/rpc.rs +++ b/core/crates/ployz-core/src/rpc.rs @@ -2271,14 +2271,12 @@ impl RpcError { Self::new(RpcErrorCode::from(APPROVAL_REQUIRED_CODE), message) } - /// `self` with `message` as its sentence. #[must_use] pub fn with_message(mut self, message: impl Into) -> Self { self.message = message.into(); self } - /// `self` with `details` as its body. #[must_use] pub fn with_details(mut self, details: Value) -> Self { self.details = details; diff --git a/core/crates/ployz/src/build/remote.rs b/core/crates/ployz/src/build/remote.rs index b383491165..5e7642d8e0 100644 --- a/core/crates/ployz/src/build/remote.rs +++ b/core/crates/ployz/src/build/remote.rs @@ -142,7 +142,6 @@ struct Admitted { active_timeout: Duration, } -/// An admission that ended in a terminal failure, and whether Device enforcement caused it. struct Refused { outcome: Outcome, by_enforcement: bool, @@ -157,10 +156,8 @@ impl From for Refused { } } -/// Why a Machine's Build capability could not be verified. #[derive(Debug, thiserror::Error)] pub(crate) enum CapabilityCheckError { - /// The Machine enforces Device approval and refused the Build. #[error("{0}")] RefusedByEnforcement(String), #[error(transparent)] diff --git a/core/crates/ployz/src/connect.rs b/core/crates/ployz/src/connect.rs index da89fd2c78..5997741b6d 100644 --- a/core/crates/ployz/src/connect.rs +++ b/core/crates/ployz/src/connect.rs @@ -1816,6 +1816,7 @@ impl From for TransportError { } None => sent.clone().and_then(crate::failure::JoinedChain::of), }; + // Tonic merges the response headers into a local failure's metadata. let marked = |key| status.metadata().contains_key(key); let routed = if !local && marked(ployz_core::rpc::TARGET_UNREACHED_KEY) { Some(Routed::Unreached) diff --git a/core/crates/ployz/src/connect/management.rs b/core/crates/ployz/src/connect/management.rs index 07877969c9..5d3faeb90a 100644 --- a/core/crates/ployz/src/connect/management.rs +++ b/core/crates/ployz/src/connect/management.rs @@ -115,8 +115,6 @@ async fn management_endpoint( })) } -/// The builder every management-side dial binds from: the relay and its trust roots, and -/// the same send setting the daemon has (#3567). pub(super) fn endpoint(secret: SecretKey, relay: &ManagementRelay) -> iroh::endpoint::Builder { IrohEndpoint::builder(presets::Minimal) .secret_key(secret) @@ -125,7 +123,6 @@ pub(super) fn endpoint(secret: SecretKey, relay: &ManagementRelay) -> iroh::endp .transport_config(transport()) } -/// Dial `machine` on `alpn` with only the relay as a hint. pub(super) async fn dial( endpoint: &IrohEndpoint, machine: PublicKey, @@ -140,8 +137,6 @@ pub(super) async fn dial( .await } -/// Sends without segmentation offload, as the daemon does: on this fresh endpoint a -/// segmented send that a NIC refuses is the command's first large request. fn transport() -> QuicTransportConfig { QuicTransportConfig::builder() .enable_segmentation_offload(SEGMENTATION_OFFLOAD) diff --git a/core/crates/ployz/src/connect/tests.rs b/core/crates/ployz/src/connect/tests.rs index 5f4577c64d..1cd480ca4a 100644 --- a/core/crates/ployz/src/connect/tests.rs +++ b/core/crates/ployz/src/connect/tests.rs @@ -2301,8 +2301,6 @@ async fn a_confirmation_telling_another_key_fails_without_inspect() { ); } -/// The six flags a `TransportError` held before it carried `Origin` and `Routed`, and the -/// predicates that read them. They are the oracle `ConnectError::facts` is held to. #[derive(Clone, Copy, Debug)] struct OldFlags { unanswered: bool, @@ -2314,8 +2312,6 @@ struct OldFlags { } impl OldFlags { - /// The combinations a status can decode to: a closed session is a local failure, and - /// an unreached mark rides on an unanswered one and is never read off a local failure. fn producible(self) -> bool { (!self.session_closed || self.local) && (!self.routed_unreached || (self.routed_unanswered && !self.local)) From 3dca3c9e961452aff49765f616989e56bb855940 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 14:59:09 +0000 Subject: [PATCH 7/7] Keep the cause-flattening scan to Failure constructors RpcError constructors carry the wire sentence, and the struct literals they replaced were never scanned. --- core/crates/ployz/tests/cli_shape.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/crates/ployz/tests/cli_shape.rs b/core/crates/ployz/tests/cli_shape.rs index 5f2466ac2c..c28b338e25 100644 --- a/core/crates/ployz/tests/cli_shape.rs +++ b/core/crates/ployz/tests/cli_shape.rs @@ -333,6 +333,9 @@ fn no_failure_message_flattens_a_cause() { let source = std::fs::read_to_string(&path).unwrap(); for constructor in CONSTRUCTORS { for (start, _) in source.match_indices(constructor) { + if source[..start].ends_with("RpcError") { + continue; + } let rest = &source[start + constructor.len()..]; let mut depth = 1; let end = rest