From 2847fa1c6e21da89931db9afa91793532f8563d6 Mon Sep 17 00:00:00 2001 From: Meshiest Date: Tue, 10 Dec 2024 10:34:48 -0500 Subject: [PATCH 1/7] fix(cli): fix consumed errors consumed by socket handler, fix empty body/plaintext error handling --- crates/cli/src/commands/env/action/mod.rs | 18 ++++++++++++++++-- crates/cli/src/commands/env/mod.rs | 16 +++++++++++----- crates/cli/src/commands/mod.rs | 5 ++++- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/crates/cli/src/commands/env/action/mod.rs b/crates/cli/src/commands/env/action/mod.rs index 9050cdd6..1a2dd779 100644 --- a/crates/cli/src/commands/env/action/mod.rs +++ b/crates/cli/src/commands/env/action/mod.rs @@ -4,7 +4,7 @@ use anyhow::Result; use clap::Parser; use clap_stdin::FileOrStdin; use reqwest::{Client, RequestBuilder, Response}; -use serde_json::json; +use serde_json::{json, Value}; use snops_cli::events::EventsClient; use snops_common::{ action_models::{AleoValue, WithTargets}, @@ -381,8 +381,22 @@ impl Action { pub async fn post_and_wait_tx(url: &str, req: RequestBuilder) -> Result<()> { use snops_common::events::EventFilter::*; + let res = req.send().await?; + + if !res.status().is_success() { + eprintln!("error {}", res.status()); + let value = match res.content_length() { + Some(0) | None => None, + _ => { + let text = res.text().await?; + Some(serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))) + } + }; + println!("{}", serde_json::to_string_pretty(&value)?); + return Ok(()); + } - let tx_id: String = req.send().await?.json().await?; + let tx_id: String = res.json().await?; eprintln!("transaction id: {tx_id}"); let mut events = EventsClient::open_with_filter(url, TransactionIs(Arc::new(tx_id))).await?; diff --git a/crates/cli/src/commands/env/mod.rs b/crates/cli/src/commands/env/mod.rs index 275971eb..bc6951d4 100644 --- a/crates/cli/src/commands/env/mod.rs +++ b/crates/cli/src/commands/env/mod.rs @@ -5,6 +5,7 @@ use anyhow::Result; use clap::{Parser, ValueHint}; use clap_stdin::FileOrStdin; use reqwest::{Client, RequestBuilder, Response}; +use serde_json::Value; use snops_cli::events::EventsClient; use snops_common::{ action_models::AleoValue, @@ -287,11 +288,16 @@ pub async fn post_and_wait(url: &str, req: RequestBuilder, env_id: EnvId) -> Res let res = req.send().await?; if !res.status().is_success() { - println!( - "{}", - serde_json::to_string_pretty(&res.json::().await?)? - ); - std::process::exit(1); + eprintln!("error: {}", res.status()); + let value = match res.content_length() { + Some(0) | None => None, + _ => { + let text = res.text().await?; + Some(serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))) + } + }; + println!("{}", serde_json::to_string_pretty(&value)?); + return Ok(()); } let mut node_map: HashMap = res.json().await?; diff --git a/crates/cli/src/commands/mod.rs b/crates/cli/src/commands/mod.rs index 4f5eaab1..79f9a2ef 100644 --- a/crates/cli/src/commands/mod.rs +++ b/crates/cli/src/commands/mod.rs @@ -90,7 +90,10 @@ impl Commands { let value = match response.content_length() { Some(0) | None => None, - _ => response.json::().await.map(Some)?, + _ => { + let text = response.text().await?; + Some(serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))) + } }; println!("{}", serde_json::to_string_pretty(&value)?); From c8c5a0c8bb786242bf3f3638e1ce3265e12784c1 Mon Sep 17 00:00:00 2001 From: Meshiest Date: Tue, 10 Dec 2024 10:59:42 -0500 Subject: [PATCH 2/7] fix(controlplane): add missing response bodies to various 404s --- crates/cli/src/commands/env/action/mod.rs | 8 +- crates/cli/src/commands/env/mod.rs | 8 +- crates/cli/src/commands/mod.rs | 15 +- crates/common/src/action_models.rs | 8 +- crates/controlplane/src/env/error.rs | 6 +- .../controlplane/src/server/actions/config.rs | 11 +- .../controlplane/src/server/actions/deploy.rs | 13 +- .../src/server/actions/execute.rs | 13 +- crates/controlplane/src/server/actions/mod.rs | 14 +- crates/controlplane/src/server/api.rs | 164 +++++++++++------- crates/controlplane/src/server/content.rs | 25 ++- 11 files changed, 180 insertions(+), 105 deletions(-) diff --git a/crates/cli/src/commands/env/action/mod.rs b/crates/cli/src/commands/env/action/mod.rs index 1a2dd779..4a29e0f7 100644 --- a/crates/cli/src/commands/env/action/mod.rs +++ b/crates/cli/src/commands/env/action/mod.rs @@ -384,12 +384,14 @@ pub async fn post_and_wait_tx(url: &str, req: RequestBuilder) -> Result<()> { let res = req.send().await?; if !res.status().is_success() { - eprintln!("error {}", res.status()); let value = match res.content_length() { - Some(0) | None => None, + Some(0) | None => { + eprintln!("error {}", res.status()); + return Ok(()); + } _ => { let text = res.text().await?; - Some(serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))) + serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text)) } }; println!("{}", serde_json::to_string_pretty(&value)?); diff --git a/crates/cli/src/commands/env/mod.rs b/crates/cli/src/commands/env/mod.rs index bc6951d4..179e170a 100644 --- a/crates/cli/src/commands/env/mod.rs +++ b/crates/cli/src/commands/env/mod.rs @@ -288,12 +288,14 @@ pub async fn post_and_wait(url: &str, req: RequestBuilder, env_id: EnvId) -> Res let res = req.send().await?; if !res.status().is_success() { - eprintln!("error: {}", res.status()); let value = match res.content_length() { - Some(0) | None => None, + Some(0) | None => { + eprintln!("error: {}", res.status()); + return Ok(()); + } _ => { let text = res.text().await?; - Some(serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))) + serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text)) } }; println!("{}", serde_json::to_string_pretty(&value)?); diff --git a/crates/cli/src/commands/mod.rs b/crates/cli/src/commands/mod.rs index 79f9a2ef..9821d1ca 100644 --- a/crates/cli/src/commands/mod.rs +++ b/crates/cli/src/commands/mod.rs @@ -84,15 +84,18 @@ impl Commands { } }?; - if !response.status().is_success() { - eprintln!("error {}", response.status()); - } - let value = match response.content_length() { - Some(0) | None => None, + Some(0) | None => { + if !response.status().is_success() { + eprintln!("error {}", response.status()); + } else { + eprintln!("{}", response.status()); + } + return Ok(()); + } _ => { let text = response.text().await?; - Some(serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))) + serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text)) } }; diff --git a/crates/common/src/action_models.rs b/crates/common/src/action_models.rs index af4b2005..5235d2a6 100644 --- a/crates/common/src/action_models.rs +++ b/crates/common/src/action_models.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use crate::{ key_source::KeySource, node_targets::{NodeTarget, NodeTargets}, - state::{CannonId, HeightRequest, InternedId}, + state::HeightRequest, }; #[derive(Deserialize, Serialize, Clone)] @@ -58,7 +58,7 @@ pub struct ExecuteAction { pub function: String, /// The cannon id of who to execute the transaction #[serde(default)] - pub cannon: CannonId, + pub cannon: String, /// The inputs to the function pub inputs: Vec, /// The optional priority fee @@ -83,7 +83,7 @@ pub struct DeployAction { pub program: String, /// The cannon id of who to execute the transaction #[serde(default)] - pub cannon: CannonId, + pub cannon: String, /// The optional priority fee #[serde(default)] pub priority_fee: Option, @@ -122,7 +122,7 @@ pub struct Reconfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub validators: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub binary: Option, + pub binary: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub private_key: Option, #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/crates/controlplane/src/env/error.rs b/crates/controlplane/src/env/error.rs index a97dfa98..d8cd4497 100644 --- a/crates/controlplane/src/env/error.rs +++ b/crates/controlplane/src/env/error.rs @@ -4,7 +4,7 @@ use snops_common::{ aot_cmds::AotCmdError, impl_into_status_code, impl_into_type_str, rpc::error::SnarkosRequestError, - state::{AgentId, CannonId, EnvId, NodeKey, TimelineId}, + state::{AgentId, EnvId, NodeKey, TimelineId}, }; use strum_macros::AsRefStr; use thiserror::Error; @@ -57,7 +57,7 @@ pub enum ExecutionError { #[error("an agent is offline, so the test cannot complete")] AgentOffline, #[error("env `{0}` not found")] - EnvNotFound(EnvId), + EnvNotFound(String), #[error(transparent)] Cannon(#[from] CannonError), #[error(transparent)] @@ -67,7 +67,7 @@ pub enum ExecutionError { #[error("env timeline is already being executed")] TimelineAlreadyStarted, #[error("unknown cannon: `{0}`")] - UnknownCannon(CannonId), + UnknownCannon(String), #[error(transparent)] AuthorizeError(#[from] AuthorizeError), #[error(transparent)] diff --git a/crates/controlplane/src/server/actions/config.rs b/crates/controlplane/src/server/actions/config.rs index 77aebab9..47528705 100644 --- a/crates/controlplane/src/server/actions/config.rs +++ b/crates/controlplane/src/server/actions/config.rs @@ -6,7 +6,7 @@ use axum::{ }; use snops_common::{ action_models::{Reconfig, WithTargets}, - state::{AgentId, AgentState, InternedId}, + state::{id_or_none, AgentId, AgentState, InternedId}, }; use super::Env; @@ -14,6 +14,7 @@ use crate::{ env::PortType, server::error::ServerError, state::{pending_reconcile_node_map, PendingAgentReconcile}, + unwrap_or_bad_request, }; pub async fn config( @@ -54,6 +55,12 @@ pub async fn config( } for WithTargets { nodes, data } in configs { + let binary = if let Some(b) = data.binary.as_ref() { + Some(unwrap_or_bad_request!("invalid binary id", id_or_none(b))) + } else { + None + }; + for agent in env.matching_agents(&nodes, &state.pool) { if let Some(h) = data.height { set_node_field!(agent, height = (height.0 + 1, h)); @@ -77,7 +84,7 @@ pub async fn config( set_node_field!(agent, validators = v.clone()); } - if let Some(b) = &data.binary { + if let Some(b) = &binary { set_node_field!(agent, binary = (*b != InternedId::default()).then_some(*b)); } diff --git a/crates/controlplane/src/server/actions/deploy.rs b/crates/controlplane/src/server/actions/deploy.rs index f316087d..d29d1bf3 100644 --- a/crates/controlplane/src/server/actions/deploy.rs +++ b/crates/controlplane/src/server/actions/deploy.rs @@ -9,7 +9,7 @@ use http::StatusCode; use snops_common::{ action_models::DeployAction, aot_cmds::AotCmd, - state::{Authorization, KeyState}, + state::{id_or_none, Authorization, KeyState}, }; use super::{execute::execute_status, Env}; @@ -18,6 +18,7 @@ use crate::{ env::{error::ExecutionError, Environment}, server::error::ServerError, state::GlobalState, + unwrap_or_bad_request, }; pub async fn deploy( @@ -26,8 +27,8 @@ pub async fn deploy( Query(query): Query, Json(action): Json, ) -> Response { - let query_addr = env.cannons.get(&action.cannon).map(|c| c.get_local_query()); - let cannon_id = action.cannon; + let cannon_id = unwrap_or_bad_request!("invalid cannon id", id_or_none(&action.cannon)); + let query_addr = env.cannons.get(&cannon_id).map(|c| c.get_local_query()); if query.is_async() { return match deploy_inner(&state, action, &env, query_addr).await { @@ -63,10 +64,14 @@ pub async fn deploy_inner( fee_record, } = action; - let Some(cannon) = env.cannons.get(&cannon_id) else { + let Some(cannon_id) = id_or_none(&cannon_id) else { return Err(ExecutionError::UnknownCannon(cannon_id)); }; + let Some(cannon) = env.cannons.get(&cannon_id) else { + return Err(ExecutionError::UnknownCannon(cannon_id.to_string())); + }; + let KeyState::Literal(resolved_pk) = env.storage.sample_keysource_pk(&private_key) else { return Err(AuthorizeError::MissingPrivateKey( format!("{}.{cannon_id} deployed program", env.id), diff --git a/crates/controlplane/src/server/actions/execute.rs b/crates/controlplane/src/server/actions/execute.rs index bac95b1b..c7faccba 100644 --- a/crates/controlplane/src/server/actions/execute.rs +++ b/crates/controlplane/src/server/actions/execute.rs @@ -11,7 +11,7 @@ use snops_common::{ action_models::{AleoValue, ExecuteAction}, aot_cmds::AotCmd, events::{Event, EventKind}, - state::{Authorization, KeyState}, + state::{id_or_none, Authorization, KeyState}, }; use tokio::select; @@ -85,8 +85,10 @@ pub async fn execute( Query(query): Query, Json(action): Json, ) -> Response { - let query_addr = env.cannons.get(&action.cannon).map(|c| c.get_local_query()); - let cannon_id = action.cannon; + let Some(cannon_id) = id_or_none(&action.cannon) else { + return ServerError::from(ExecutionError::UnknownCannon(action.cannon)).into_response(); + }; + let query_addr = env.cannons.get(&cannon_id).map(|c| c.get_local_query()); if query.is_async() { return match execute_inner(&state, action, &env, query_addr).await { @@ -123,9 +125,12 @@ pub async fn execute_inner( priority_fee, fee_record, } = action; + let Some(cannon_id) = id_or_none(&cannon_id) else { + return Err(ExecutionError::UnknownCannon(cannon_id)); + }; let Some(cannon) = env.cannons.get(&cannon_id) else { - return Err(ExecutionError::UnknownCannon(cannon_id)); + return Err(ExecutionError::UnknownCannon(cannon_id.to_string())); }; let KeyState::Literal(resolved_pk) = env.storage.sample_keysource_pk(&private_key) else { diff --git a/crates/controlplane/src/server/actions/mod.rs b/crates/controlplane/src/server/actions/mod.rs index 2a087d52..d269e173 100644 --- a/crates/controlplane/src/server/actions/mod.rs +++ b/crates/controlplane/src/server/actions/mod.rs @@ -6,9 +6,10 @@ use axum::{ routing::post, Router, }; -use http::{request::Parts, StatusCode}; +use http::request::Parts; use snops_common::state::{id_or_none, EnvId}; +use super::error::ServerError; use crate::{env::Environment, state::AppState}; mod config; @@ -73,12 +74,13 @@ impl FromRequestParts for Env { let params = CommonParams::from_request_parts(parts, state).await?; // get environment - let env_id = - id_or_none(¶ms.env_id).ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; + let env_id = id_or_none(¶ms.env_id).ok_or_else(|| { + ServerError::NotFound("unknown environment id".to_owned()).into_response() + })?; - let env = state - .get_env(env_id) - .ok_or_else(|| StatusCode::NOT_FOUND.into_response())?; + let env = state.get_env(env_id).ok_or_else(|| { + ServerError::NotFound("environment not found".to_owned()).into_response() + })?; Ok(Self { env, diff --git a/crates/controlplane/src/server/api.rs b/crates/controlplane/src/server/api.rs index 128ca728..18f50821 100644 --- a/crates/controlplane/src/server/api.rs +++ b/crates/controlplane/src/server/api.rs @@ -32,10 +32,20 @@ use crate::{ #[macro_export] macro_rules! unwrap_or_not_found { - ($e:expr) => { + ($s:expr, $e:expr) => { match $e { Some(v) => v, - None => return ::axum::http::StatusCode::NOT_FOUND.into_response(), + None => return ServerError::NotFound($s.to_owned()).into_response(), + } + }; +} + +#[macro_export] +macro_rules! unwrap_or_bad_request { + ($s:expr, $e:expr) => { + match $e { + Some(v) => v, + None => return ServerError::BadRequest($s.to_owned()).into_response(), } }; } @@ -94,15 +104,15 @@ async fn set_agent_log_level( state: State, Path((id, level)): Path<(String, String)>, ) -> Response { - let id = unwrap_or_not_found!(id_or_none(&id)); - let agent = unwrap_or_not_found!(state.pool.get(&id)); + let id = unwrap_or_not_found!("unknown agent id", id_or_none(&id)); + let agent = unwrap_or_not_found!("agent not found", state.pool.get(&id)); tracing::debug!("attempting to set agent log level to {level} for agent {id}"); - let Some(rpc) = agent.rpc() else { + let Some(rpc) = agent.client_owned() else { return StatusCode::SERVICE_UNAVAILABLE.into_response(); }; - let Err(e) = rpc.set_log_level(tarpc::context::current(), level).await else { + let Err(e) = rpc.0.set_log_level(tarpc::context::current(), level).await else { return status_ok(); }; @@ -113,8 +123,8 @@ async fn set_aot_log_level( state: State, Path((id, verbosity)): Path<(String, u8)>, ) -> Response { - let id = unwrap_or_not_found!(id_or_none(&id)); - let agent = unwrap_or_not_found!(state.pool.get(&id)); + let id = unwrap_or_not_found!("unknown agent id", id_or_none(&id)); + let agent = unwrap_or_not_found!("agent not found", state.pool.get(&id)); tracing::debug!("attempting to set aot log verbosity to {verbosity} for agent {id}"); let Some(rpc) = agent.rpc() else { @@ -150,17 +160,20 @@ async fn set_log_level(Path(level): Path, state: State) -> Res } async fn get_env_info(Path(env_id): Path, state: State) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); Json(env.info(&state)).into_response() } async fn get_latest_height(Path(env_id): Path, state: State) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); - let cannon = unwrap_or_not_found!(env.get_cannon(CannonId::default())); + let cannon = unwrap_or_not_found!( + "default cannon not found", + env.get_cannon(CannonId::default()) + ); match &cannon.source.query { QueryTarget::Local(_qs) => StatusCode::NOT_IMPLEMENTED.into_response(), @@ -177,8 +190,9 @@ async fn get_latest_height(Path(env_id): Path, state: State) - } async fn get_env_block_info(Path(env_id): Path, state: State) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let block_info = unwrap_or_not_found!(state.get_env_block_info(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let block_info = + unwrap_or_not_found!("environment not found", state.get_env_block_info(env_id)); Json(block_info).into_response() } @@ -187,15 +201,15 @@ async fn get_env_balance( Path((env_id, keysource)): Path<(String, KeySource)>, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); let KeyState::Literal(key) = env.storage.sample_keysource_addr(&keysource) else { return ServerError::NotFound(format!("keysource pubkey {keysource}")).into_response(); }; let Some(cannon) = env.get_cannon(CannonId::default()) else { - return ServerError::NotFound("cannon not found".to_owned()).into_response(); + return ServerError::NotFound("default cannon not found".to_owned()).into_response(); }; match &cannon.source.query { @@ -233,9 +247,12 @@ async fn get_block( Path((env_id, height_or_hash)): Path<(String, String)>, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); - let cannon = unwrap_or_not_found!(env.get_cannon(CannonId::default())); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); + let cannon = unwrap_or_not_found!( + "default cannon not found", + env.get_cannon(CannonId::default()) + ); match &cannon.source.query { QueryTarget::Local(_qs) => StatusCode::NOT_IMPLEMENTED.into_response(), @@ -259,9 +276,12 @@ async fn get_tx_blockhash( Path((env_id, transaction)): Path<(String, String)>, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); - let cannon = unwrap_or_not_found!(env.get_cannon(CannonId::default())); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); + let cannon = unwrap_or_not_found!( + "default cannon not found", + env.get_cannon(CannonId::default()) + ); match &cannon.source.query { QueryTarget::Local(_qs) => StatusCode::NOT_IMPLEMENTED.into_response(), @@ -285,9 +305,12 @@ async fn get_tx( Path((env_id, transaction)): Path<(String, String)>, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); - let cannon = unwrap_or_not_found!(env.get_cannon(CannonId::default())); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); + let cannon = unwrap_or_not_found!( + "default cannon not found", + env.get_cannon(CannonId::default()) + ); match &cannon.source.query { QueryTarget::Local(_qs) => StatusCode::NOT_IMPLEMENTED.into_response(), @@ -322,15 +345,15 @@ fn status_ok() -> Response { } async fn get_agent(state: State, Path(id): Path) -> Response { - let id = unwrap_or_not_found!(id_or_none(&id)); - let agent = unwrap_or_not_found!(state.pool.get(&id)); + let id = unwrap_or_not_found!("unknown agent id", id_or_none(&id)); + let agent = unwrap_or_not_found!("agent not found", state.pool.get(&id)); Json(AgentStatusResponse::from(agent.value())).into_response() } async fn get_agent_status(state: State, Path(id): Path) -> Response { - let id = unwrap_or_not_found!(id_or_none(&id)); - let agent = unwrap_or_not_found!(state.pool.get(&id)); + let id = unwrap_or_not_found!("unknown agent id", id_or_none(&id)); + let agent = unwrap_or_not_found!("agent not found", state.pool.get(&id)); let Some(rpc) = agent.rpc() else { return StatusCode::SERVICE_UNAVAILABLE.into_response(); @@ -343,8 +366,11 @@ async fn get_agent_status(state: State, Path(id): Path) -> Res } async fn kill_agent(state: State, Path(id): Path) -> Response { - let id = unwrap_or_not_found!(id_or_none(&id)); - let client = unwrap_or_not_found!(state.pool.get(&id).and_then(|a| a.client_owned())); + let id = unwrap_or_not_found!("unknown agent id", id_or_none(&id)); + let client = unwrap_or_not_found!( + "agent not found", + state.pool.get(&id).and_then(|a| a.client_owned()) + ); if let Err(e) = client.0.kill(context::current()).await { tracing::error!("failed to kill agent {id}: {e}"); @@ -359,14 +385,15 @@ async fn kill_agent(state: State, Path(id): Path) -> Response } async fn get_agent_tps(state: State, Path(id): Path) -> Response { - let id = unwrap_or_not_found!(id_or_none(&id)); - let agent = unwrap_or_not_found!(state.pool.get(&id)); + let id = unwrap_or_not_found!("unknown agent id", id_or_none(&id)); + let agent = unwrap_or_not_found!("agent not found", state.pool.get(&id)); - let Some(rpc) = agent.rpc() else { + let Some(rpc) = agent.client_owned() else { return StatusCode::SERVICE_UNAVAILABLE.into_response(); }; match rpc + .0 .get_metric(tarpc::context::current(), AgentMetric::Tps) .await { @@ -379,7 +406,7 @@ async fn get_program( Path((env_id, program)): Path<(String, String)>, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); match state .snarkos_get::(env_id, format!("/program/{program}"), &NodeTargets::ALL) .await @@ -400,9 +427,12 @@ async fn get_mapping_value( Query(query): Query, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); - let cannon = unwrap_or_not_found!(env.get_cannon(CannonId::default())); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); + let cannon = unwrap_or_not_found!( + "default cannon not found", + env.get_cannon(CannonId::default()) + ); let url = match (query.key, query.keysource) { (Some(key), None) => { @@ -416,11 +446,8 @@ async fn get_mapping_value( format!("/program/{program}/mapping/{mapping}/{key}",) } _ => { - return ( - StatusCode::BAD_REQUEST, - Json(json!({"error": "either key or key_source must be provided"})), - ) - .into_response(); + return ServerError::BadRequest("either key or key_source must be provided".to_owned()) + .into_response() } }; @@ -442,9 +469,12 @@ async fn get_mappings( Path((env_id, program)): Path<(String, String)>, state: State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); - let cannon = unwrap_or_not_found!(env.get_cannon(CannonId::default())); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); + let cannon = unwrap_or_not_found!( + "default cannon not found", + env.get_cannon(CannonId::default()) + ); match &cannon.source.query { QueryTarget::Local(_qs) => StatusCode::NOT_IMPLEMENTED.into_response(), @@ -517,15 +547,16 @@ async fn get_env_list(State(state): State) -> Response { } async fn get_env_topology(Path(env_id): Path, State(state): State) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); let mut internal = HashMap::new(); let mut external = HashMap::new(); for (nk, peer) in env.node_peers.iter() { - // safe to unwrap because we know the agent exists - let node_state = env.node_states.get(nk).unwrap().clone(); + let Some(node_state) = env.node_states.get(nk) else { + continue; + }; match peer { EnvPeer::Internal(id) => { internal.insert(*id, node_state); @@ -546,15 +577,16 @@ async fn get_env_topology_resolved( Path(env_id): Path, State(state): State, ) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); let mut resolved = HashMap::new(); for (_, peer) in env.node_peers.iter() { - // safe to unwrap because we know the agent exists if let EnvPeer::Internal(id) = peer { - let agent = state.pool.get(id).unwrap(); + let Some(agent) = state.pool.get(id) else { + continue; + }; match agent.state().clone() { AgentState::Inventory => continue, AgentState::Node(_, state) => { @@ -569,8 +601,8 @@ async fn get_env_topology_resolved( /// Get a map of node keys to agent ids async fn get_env_agents(Path(env_id): Path, State(state): State) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); Json( env.node_peers @@ -589,11 +621,15 @@ async fn get_env_agent_key( Path((env_id, node_type, node_key)): Path<(String, String, String)>, State(state): State, ) -> Response { - let node_key = unwrap_or_not_found!(NodeKey::from_str(&format!("{node_type}/{node_key}")).ok()); - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); - let env = unwrap_or_not_found!(state.get_env(env_id)); - let agent_id = unwrap_or_not_found!(env.get_agent_by_key(&node_key)); - let agent = unwrap_or_not_found!(state.pool.get(&agent_id)); + let node_key = unwrap_or_bad_request!( + "invalid node key", + NodeKey::from_str(&format!("{node_type}/{node_key}")).ok() + ); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); + let env = unwrap_or_not_found!("environment not found", state.get_env(env_id)); + let agent_id = + unwrap_or_not_found!("node found in environment", env.get_agent_by_key(&node_key)); + let agent = unwrap_or_not_found!("agent not found", state.pool.get(&agent_id)); Json(AgentStatusResponse::from(agent.value())).into_response() } @@ -617,7 +653,7 @@ async fn post_env_apply( } async fn delete_env(Path(env_id): Path, State(state): State) -> Response { - let env_id = unwrap_or_not_found!(id_or_none(&env_id)); + let env_id = unwrap_or_not_found!("unknown environment id", id_or_none(&env_id)); match Environment::cleanup(env_id, &state).await { Ok(_) => status_ok(), diff --git a/crates/controlplane/src/server/content.rs b/crates/controlplane/src/server/content.rs index c6066ee5..fe2b8e6b 100644 --- a/crates/controlplane/src/server/content.rs +++ b/crates/controlplane/src/server/content.rs @@ -10,7 +10,7 @@ use axum::{ use http::{StatusCode, Uri}; use snops_common::{ binaries::{BinaryEntry, BinarySource}, - state::{InternedId, NetworkId, StorageId}, + state::{id_or_none, InternedId, NetworkId}, }; use tower::Service; use tower_http::services::ServeFile; @@ -22,7 +22,7 @@ use crate::{ }, server::error::ServerError, state::{AppState, GlobalState}, - unwrap_or_not_found, + unwrap_or_bad_request, unwrap_or_not_found, }; async fn not_found(uri: Uri, res: Response) -> Response { @@ -75,11 +75,18 @@ pub(super) async fn init_routes(state: &GlobalState) -> Router { /// Serve a binary from the storage or a redirect to the binary async fn serve_binary( - Path((network, storage_id, binary_id)): Path<(NetworkId, StorageId, InternedId)>, + Path((network, storage_id, binary_id)): Path<(NetworkId, String, String)>, State(state): State, req: Request, ) -> Response { - let storage = unwrap_or_not_found!(state.storage.get(&(network, storage_id))).clone(); + let storage_id = unwrap_or_bad_request!("invalid storage id", id_or_none(&storage_id)); + let binary_id = unwrap_or_bad_request!("invalid binary id", id_or_none(&binary_id)); + + let storage = unwrap_or_not_found!( + "storage not found", + state.storage.get(&(network, storage_id)) + ) + .clone(); match storage.resolve_binary_entry(binary_id) { Ok((id, entry)) => respond_from_entry(id, entry, req).await, @@ -99,11 +106,17 @@ async fn respond_from_entry(id: InternedId, entry: &BinaryEntry, req: Request) - } async fn serve_file( - Path((network, storage_id, file)): Path<(NetworkId, StorageId, String)>, + Path((network, storage_id, file)): Path<(NetworkId, String, String)>, State(state): State, req: Request, ) -> Response { - let storage = unwrap_or_not_found!(state.storage.get(&(network, storage_id))).clone(); + let storage_id = unwrap_or_bad_request!("invalid storage id", id_or_none(&storage_id)); + + let storage = unwrap_or_not_found!( + "storage not found", + state.storage.get(&(network, storage_id)) + ) + .clone(); match file.as_str() { // ensure genesis is only served if native genesis is disabled From cf9869a30bef1beedd60a2d49c3c1273d533ccb4 Mon Sep 17 00:00:00 2001 From: Meshiest Date: Tue, 10 Dec 2024 11:50:58 -0500 Subject: [PATCH 3/7] fix(controlplane): fix default cannon value for action api --- crates/common/src/action_models.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/common/src/action_models.rs b/crates/common/src/action_models.rs index 5235d2a6..bdf26db3 100644 --- a/crates/common/src/action_models.rs +++ b/crates/common/src/action_models.rs @@ -41,6 +41,10 @@ fn credits_aleo() -> String { "credits.aleo".to_owned() } +fn default_str() -> String { + "default".to_owned() +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub struct ExecuteAction { @@ -57,7 +61,7 @@ pub struct ExecuteAction { /// The function to call pub function: String, /// The cannon id of who to execute the transaction - #[serde(default)] + #[serde(default = "default_str")] pub cannon: String, /// The inputs to the function pub inputs: Vec, @@ -82,7 +86,7 @@ pub struct DeployAction { /// The program to deploy pub program: String, /// The cannon id of who to execute the transaction - #[serde(default)] + #[serde(default = "default_str")] pub cannon: String, /// The optional priority fee #[serde(default)] From 285b4afc4542d73282d6441411a0be3e27bd68d0 Mon Sep 17 00:00:00 2001 From: Meshiest Date: Tue, 10 Dec 2024 12:53:40 -0500 Subject: [PATCH 4/7] fix(agent): fix request failure errors not providing more information --- crates/agent/src/rpc/control.rs | 31 +++++++++++++---------- crates/aot/src/runner/rpc/node.rs | 16 +++++++----- crates/common/src/rpc/error.rs | 4 +-- crates/controlplane/src/cannon/context.rs | 6 ++--- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/crates/agent/src/rpc/control.rs b/crates/agent/src/rpc/control.rs index 285a7a1c..85cd181f 100644 --- a/crates/agent/src/rpc/control.rs +++ b/crates/agent/src/rpc/control.rs @@ -177,7 +177,7 @@ impl AgentService for AgentRpcServer { .state .get_env_info(env_id) .await - .map_err(|_| AgentError::FailedToMakeRequest)? + .map_err(|e| AgentError::FailedToGetEnvInfo(e.to_string()))? .network; let url = format!( @@ -186,26 +186,31 @@ impl AgentService for AgentRpcServer { self.state.cli.ports.rest ); let response = reqwest::Client::new() - .post(url) + .post(&url) .header("Content-Type", "application/json") .body(tx) .send() .await - .map_err(|_| AgentError::FailedToMakeRequest)?; + .map_err(|e| AgentError::FailedToMakeRequest(format!("{url}: {e:?}")))?; let status = response.status(); if status.is_success() { - Ok(()) + return Ok(()); // transaction already exists so this is technically a success - } else if status.is_server_error() - && response - .text() - .await - .ok() + } + + let text = response.text().await.ok(); + + if status.is_server_error() + && text + .as_ref() .is_some_and(|text| text.contains("exists in the ledger")) { - return Ok(()); + Ok(()) } else { - Err(AgentError::FailedToMakeRequest) + Err(AgentError::FailedToMakeRequest(format!( + "{url}: {status:?} {}", + text.unwrap_or_else(|| "no error message".to_string()) + ))) } } @@ -329,7 +334,7 @@ impl AgentService for AgentRpcServer { .ok_or(AgentError::NodeClientNotSet)? .get_block_lite(ctx, block_hash) .await - .map_err(|_| AgentError::FailedToMakeRequest)? + .map_err(|e| AgentError::FailedToMakeRequest(format!("block info: {e:?}")))? } async fn find_transaction( @@ -343,7 +348,7 @@ impl AgentService for AgentRpcServer { .ok_or(AgentError::NodeClientNotSet)? .find_transaction(context, tx_id) .await - .map_err(|_| AgentError::FailedToMakeRequest)? + .map_err(|e| AgentError::FailedToMakeRequest(format!("find tx: {e:?}")))? } async fn get_status(self, ctx: Context) -> Result { diff --git a/crates/aot/src/runner/rpc/node.rs b/crates/aot/src/runner/rpc/node.rs index 924c8653..451705ee 100644 --- a/crates/aot/src/runner/rpc/node.rs +++ b/crates/aot/src/runner/rpc/node.rs @@ -62,7 +62,9 @@ impl NodeService for NodeRpcServer { block_db .find_block_hash(&tx_id) - .map_err(|_| AgentError::FailedToMakeRequest) + .map_err(|e| { + AgentError::FailedToMakeRequest(format!("block hash for tx {tx_id}: {e:?}")) + }) .map(|hash| hash.map(|hash| hash.to_string())) } @@ -80,9 +82,9 @@ impl NodeService for NodeRpcServer { .parse() .map_err(|_| AgentError::InvalidBlockHash)?; - let Some(height) = block_db - .get_block_height(&hash) - .map_err(|_| AgentError::FailedToMakeRequest)? + let Some(height) = block_db.get_block_height(&hash).map_err(|e| { + AgentError::FailedToMakeRequest(format!("block height for hash {hash}: {e:?}")) + })? else { return Ok(None); }; @@ -91,9 +93,9 @@ impl NodeService for NodeRpcServer { return Ok(None); }; - let Some(transactions) = block_db - .get_block_transactions(&hash) - .map_err(|_| AgentError::FailedToMakeRequest)? + let Some(transactions) = block_db.get_block_transactions(&hash).map_err(|e| { + AgentError::FailedToMakeRequest(format!("transactions for height {height}: {e:?}")) + })? else { return Ok(None); }; diff --git a/crates/common/src/rpc/error.rs b/crates/common/src/rpc/error.rs index 3a6ecb24..4977c33a 100644 --- a/crates/common/src/rpc/error.rs +++ b/crates/common/src/rpc/error.rs @@ -70,8 +70,8 @@ pub enum AgentError { InvalidState, #[error("failed to parse json")] FailedToParseJson, - #[error("failed to make a request")] - FailedToMakeRequest, + #[error("failed to make a request: {0}")] + FailedToMakeRequest(String), #[error("failed to get env info: {0}")] FailedToGetEnvInfo(String), #[error("failed to spawn a process")] diff --git a/crates/controlplane/src/cannon/context.rs b/crates/controlplane/src/cannon/context.rs index 13833bc8..33e89014 100644 --- a/crates/controlplane/src/cannon/context.rs +++ b/crates/controlplane/src/cannon/context.rs @@ -308,7 +308,7 @@ impl ExecutionContext { }; if let Err(e) = client.broadcast_tx(tx_str.clone()).await { - warn!("cannon {env_id}.{cannon_id} failed to broadcast transaction to agent {id}: {e}"); + warn!("cannon {env_id}.{cannon_id} failed to broadcast transaction to agent {id}: {e:?}"); continue; } @@ -332,7 +332,7 @@ impl ExecutionContext { match res { Err(e) => { - warn!("cannon {env_id}.{cannon_id} failed to broadcast transaction to {addr}: {e}"); + warn!("cannon {env_id}.{cannon_id} failed to broadcast transaction to {addr}: {e:?}"); continue; } Ok(req) => { @@ -350,7 +350,7 @@ impl ExecutionContext { return Ok(tx_id); } - warn!("cannon {env_id}.{cannon_id} failed to broadcast transaction to {addr}: {}", status); + warn!("cannon {env_id}.{cannon_id} failed to broadcast transaction to {addr}: {status}"); continue; } } From 83feaf0b32667f007e99f74caab367dfc0303411 Mon Sep 17 00:00:00 2001 From: gluax <16431709+gluax@users.noreply.github.com> Date: Sat, 7 Dec 2024 07:35:39 -0800 Subject: [PATCH 5/7] fix: common compiling, clippy and fmt fixes --- .github/workflows/pr.yml | 4 ++-- crates/agent/src/reconcile/storage.rs | 2 +- crates/aot/src/accounts.rs | 2 +- crates/common/Cargo.toml | 4 ++-- crates/common/src/key_source.rs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4bc4dbe5..ca6344f4 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -5,8 +5,8 @@ on: env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 - # https://releases.rs/docs/1.82.0/ release date - NIGHTLY_TOOLCHAIN: nightly-2024-10-17 + # https://releases.rs/docs/1.83.0/ release date + NIGHTLY_TOOLCHAIN: nightly-2024-11-28 # Cancel in progress workflows on pull_requests. # https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value diff --git a/crates/agent/src/reconcile/storage.rs b/crates/agent/src/reconcile/storage.rs index 90383671..a28ce7f7 100644 --- a/crates/agent/src/reconcile/storage.rs +++ b/crates/agent/src/reconcile/storage.rs @@ -205,7 +205,7 @@ pub struct LedgerReconciler<'a> { pub modify_handle: &'a mut Option<(AbortHandle, Arc>>)>, } -impl<'a> LedgerReconciler<'a> { +impl LedgerReconciler<'_> { pub fn untar_paths(&self) -> (PathBuf, &'static str) { if self.env_info.storage.persist { ( diff --git a/crates/aot/src/accounts.rs b/crates/aot/src/accounts.rs index 61d266ab..02497dbd 100644 --- a/crates/aot/src/accounts.rs +++ b/crates/aot/src/accounts.rs @@ -38,7 +38,7 @@ pub const BECH32M_CHARSET: &str = "0123456789acdefghjklmnpqrstuvwxyz"; #[derive(Clone, Copy)] struct VanityCheck<'a>(&'a [bech32::u5]); -impl<'a> bech32::WriteBase32 for VanityCheck<'a> { +impl bech32::WriteBase32 for VanityCheck<'_> { type Err = bool; fn write_u5(&mut self, data: bech32::u5) -> std::result::Result<(), Self::Err> { diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index 3d5268da..8e76c368 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -19,7 +19,7 @@ clap.workspace = true clap_mangen = { workspace = true, optional = true } clap-markdown = { workspace = true, optional = true } futures.workspace = true -http.workspace = true +http = { workspace = true, features = ["std"] } indexmap = { workspace = true, features = ["std", "serde"] } lasso.workspace = true lazy_static.workspace = true @@ -36,7 +36,7 @@ tarpc.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["process"] } tracing.workspace = true -url.workspace = true +url = { workspace = true, features = ["serde"] } wildmatch.workspace = true [dev-dependencies] diff --git a/crates/common/src/key_source.rs b/crates/common/src/key_source.rs index 9b9d6a7d..ecd2e044 100644 --- a/crates/common/src/key_source.rs +++ b/crates/common/src/key_source.rs @@ -45,7 +45,7 @@ impl<'de> Deserialize<'de> for KeySource { { struct KeySourceVisitor; - impl<'de> Visitor<'de> for KeySourceVisitor { + impl Visitor<'_> for KeySourceVisitor { type Value = KeySource; fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { From 51ef740bb84a5d86c043270739478b443ba068f1 Mon Sep 17 00:00:00 2001 From: gluax <16431709+gluax@users.noreply.github.com> Date: Sat, 7 Dec 2024 07:57:01 -0800 Subject: [PATCH 6/7] fix(ci): don't fail if module has no tests --- .github/workflows/pr.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ca6344f4..f8ef3b2b 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -135,38 +135,38 @@ jobs: - name: 🧪 Test All if: steps.changes.outputs.top_toml == 'true' - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run --all --verbose --fail-fast --all-features --exclude snops-agent --exclude xtask + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run --all --verbose --fail-fast --all-features --exclude snops-agent --exclude xtask --no-tests=warn - name: 🧪 Test Aot if: steps.changes.outputs.aot == 'true' && steps.changes.outputs.top_toml == 'false' - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snarkos-aot --verbose --fail-fast --all-features + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snarkos-aot --verbose --fail-fast --all-features --no-tests=warn - name: 🧪 Test Checkpoint # env: # RUSTFLAGS: -Zcodegen-backend=cranelift if: steps.changes.outputs.checkpoint == 'true' && steps.changes.outputs.top_toml == 'false' - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-checkpoint --verbose --fail-fast --all-features + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-checkpoint --verbose --fail-fast --all-features --no-tests=warn - name: 🧪 Test Common # env: # RUSTFLAGS: -Zcodegen-backend=cranelift if: steps.changes.outputs.common == 'true' && steps.changes.outputs.top_toml == 'false' - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-common --verbose --fail-fast --all-features + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-common --verbose --fail-fast --all-features --no-tests=warn - name: 🧪 Test Control Plane # env: # RUSTFLAGS: -Zcodegen-backend=cranelift if: (steps.changes.outputs.control_plane == 'true' || steps.changes.outputs.common == 'true') && steps.changes.outputs.top_toml == 'false' - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops --verbose --fail-fast --all-features + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops --verbose --fail-fast --all-features --no-tests=warn - name: 🧪 Test Agent # env: # RUSTFLAGS: "" if: (steps.changes.outputs.agent == 'true' || steps.changes.outputs.common == 'true') - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-agent --verbose --fail-fast --all-features + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-agent --verbose --fail-fast --all-features --no-tests=warn - name: 🧪 Test Scli # env: # RUSTFLAGS: -Zcodegen-backend=cranelift if: (steps.changes.outputs.scli == 'true' || steps.changes.outputs.common == 'true') && steps.changes.outputs.top_toml == 'false' - run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-cli --verbose --fail-fast --all-features + run: cargo +${{ env.NIGHTLY_TOOLCHAIN }} nextest run -p snops-cli --verbose --fail-fast --all-features --no-tests=warn From 17b552d9934c393c25493adadacb72641569ffed Mon Sep 17 00:00:00 2001 From: Meshiest Date: Tue, 10 Dec 2024 22:18:47 -0500 Subject: [PATCH 7/7] fix(controlplane): fix inconsistent error codes for invalid interned ids --- crates/controlplane/src/server/actions/config.rs | 4 ++-- crates/controlplane/src/server/actions/deploy.rs | 4 ++-- crates/controlplane/src/server/content.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/controlplane/src/server/actions/config.rs b/crates/controlplane/src/server/actions/config.rs index 47528705..bb7bfc6d 100644 --- a/crates/controlplane/src/server/actions/config.rs +++ b/crates/controlplane/src/server/actions/config.rs @@ -14,7 +14,7 @@ use crate::{ env::PortType, server::error::ServerError, state::{pending_reconcile_node_map, PendingAgentReconcile}, - unwrap_or_bad_request, + unwrap_or_not_found, }; pub async fn config( @@ -56,7 +56,7 @@ pub async fn config( for WithTargets { nodes, data } in configs { let binary = if let Some(b) = data.binary.as_ref() { - Some(unwrap_or_bad_request!("invalid binary id", id_or_none(b))) + Some(unwrap_or_not_found!("unknown binary id", id_or_none(b))) } else { None }; diff --git a/crates/controlplane/src/server/actions/deploy.rs b/crates/controlplane/src/server/actions/deploy.rs index d29d1bf3..97067447 100644 --- a/crates/controlplane/src/server/actions/deploy.rs +++ b/crates/controlplane/src/server/actions/deploy.rs @@ -18,7 +18,7 @@ use crate::{ env::{error::ExecutionError, Environment}, server::error::ServerError, state::GlobalState, - unwrap_or_bad_request, + unwrap_or_not_found, }; pub async fn deploy( @@ -27,7 +27,7 @@ pub async fn deploy( Query(query): Query, Json(action): Json, ) -> Response { - let cannon_id = unwrap_or_bad_request!("invalid cannon id", id_or_none(&action.cannon)); + let cannon_id = unwrap_or_not_found!("unknown cannon id", id_or_none(&action.cannon)); let query_addr = env.cannons.get(&cannon_id).map(|c| c.get_local_query()); if query.is_async() { diff --git a/crates/controlplane/src/server/content.rs b/crates/controlplane/src/server/content.rs index fe2b8e6b..5c4699f0 100644 --- a/crates/controlplane/src/server/content.rs +++ b/crates/controlplane/src/server/content.rs @@ -79,8 +79,8 @@ async fn serve_binary( State(state): State, req: Request, ) -> Response { - let storage_id = unwrap_or_bad_request!("invalid storage id", id_or_none(&storage_id)); - let binary_id = unwrap_or_bad_request!("invalid binary id", id_or_none(&binary_id)); + let storage_id = unwrap_or_not_found!("unknown storage id", id_or_none(&storage_id)); + let binary_id = unwrap_or_not_found!("unknown binary id", id_or_none(&binary_id)); let storage = unwrap_or_not_found!( "storage not found",