fix(controlplane): add missing response bodies to various 404s

monadicus · Dec 10, 2024 · c8c5a0c · c8c5a0c
1 parent 2847fa1
commit c8c5a0c
Show file tree

Hide file tree

Showing 11 changed files with 180 additions and 105 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<AleoValue>,
     /// 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<u64>,
@@ -122,7 +122,7 @@ pub struct Reconfig {
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub validators: Option<NodeTargets>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub binary: Option<InternedId>,
+    pub binary: Option<String>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub private_key: Option<KeySource>,
     #[serde(default, skip_serializing_if = "Option::is_none")]

diff --git 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
@@ -6,14 +6,15 @@ use axum::{
 };
 use snops_common::{
     action_models::{Reconfig, WithTargets},
-    state::{AgentId, AgentState, InternedId},
+    state::{id_or_none, AgentId, AgentState, InternedId},
 };
 
 use super::Env;
 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
@@ -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<AuthQuery>,
     Json(action): Json<DeployAction>,
 ) -> 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
@@ -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<AuthQuery>,
     Json(action): Json<ExecuteAction>,
 ) -> 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
@@ -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<AppState> for Env {
         let params = CommonParams::from_request_parts(parts, state).await?;
 
         // get environment
-        let env_id =
-            id_or_none(&params.env_id).ok_or_else(|| StatusCode::NOT_FOUND.into_response())?;
+        let env_id = id_or_none(&params.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,