Merge pull request #321 from monadicus/fix-api-errors

fix(controlplane,cli): Add missing response bodies to many 404 errors, properly handle response bodies and errors on CLI

Author: Meshiest

.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
@@ -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

crates/agent/src/reconcile/storage.rs

@@ -205,7 +205,7 @@ pub struct LedgerReconciler<'a> {
     pub modify_handle: &'a mut Option<(AbortHandle, Arc<Mutex<Option<LedgerModifyResult>>>)>,
 }
 
-impl<'a> LedgerReconciler<'a> {
+impl LedgerReconciler<'_> {
     pub fn untar_paths(&self) -> (PathBuf, &'static str) {
         if self.env_info.storage.persist {
             (

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<AgentStatus, AgentError> {

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> {

crates/aot/src/runner/rpc/node.rs

@@ -62,7 +62,9 @@ impl<N: Network> NodeService for NodeRpcServer<N> {
 
         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<N: Network> NodeService for NodeRpcServer<N> {
             .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<N: Network> NodeService for NodeRpcServer<N> {
             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);
         };

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,24 @@ impl Action {
 
 pub async fn post_and_wait_tx(url: &str, req: RequestBuilder) -> Result<()> {
     use snops_common::events::EventFilter::*;
+    let res = req.send().await?;
 
-    let tx_id: String = req.send().await?.json().await?;
+    if !res.status().is_success() {
+        let value = match res.content_length() {
+            Some(0) | None => {
+                eprintln!("error {}", res.status());
+                return Ok(());
+            }
+            _ => {
+                let text = res.text().await?;
+                serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))
+            }
+        };
+        println!("{}", serde_json::to_string_pretty(&value)?);
+        return Ok(());
+    }
+
+    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?;

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,18 @@ 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::<serde_json::Value>().await?)?
-        );
-        std::process::exit(1);
+        let value = match res.content_length() {
+            Some(0) | None => {
+                eprintln!("error: {}", res.status());
+                return Ok(());
+            }
+            _ => {
+                let text = res.text().await?;
+                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<NodeKey, AgentId> = res.json().await?;

crates/cli/src/commands/mod.rs

@@ -84,13 +84,19 @@ impl Commands {
             }
         }?;
 
-        if !response.status().is_success() {
-            eprintln!("error {}", response.status());
-        }
-
         let value = match response.content_length() {
-            Some(0) | None => None,
-            _ => response.json::<Value>().await.map(Some)?,
+            Some(0) | None => {
+                if !response.status().is_success() {
+                    eprintln!("error {}", response.status());
+                } else {
+                    eprintln!("{}", response.status());
+                }
+                return Ok(());
+            }
+            _ => {
+                let text = response.text().await?;
+                serde_json::from_str(&text).unwrap_or_else(|_| Value::String(text))
+            }
         };
 
         println!("{}", serde_json::to_string_pretty(&value)?);

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]

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)]
@@ -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,8 +61,8 @@ pub struct ExecuteAction {
     /// The function to call
     pub function: String,
     /// The cannon id of who to execute the transaction
-    #[serde(default)]
-    pub cannon: CannonId,
+    #[serde(default = "default_str")]
+    pub cannon: String,
     /// The inputs to the function
     pub inputs: Vec<AleoValue>,
     /// The optional priority fee
@@ -82,8 +86,8 @@ pub struct DeployAction {
     /// The program to deploy
     pub program: String,
     /// The cannon id of who to execute the transaction
-    #[serde(default)]
-    pub cannon: CannonId,
+    #[serde(default = "default_str")]
+    pub cannon: String,
     /// The optional priority fee
     #[serde(default)]
     pub priority_fee: Option<u64>,
@@ -122,7 +126,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")]