Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Dec 11, 2024
18 changes: 9 additions & 9 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion crates/agent/src/reconcile/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
(
Expand Down
31 changes: 18 additions & 13 deletions crates/agent/src/rpc/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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())
)))
}
}

Expand Down Expand Up @@ -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(
Expand All @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion crates/aot/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
16 changes: 9 additions & 7 deletions crates/aot/src/runner/rpc/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}

Expand All @@ -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);
};
Expand All @@ -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);
};
Expand Down
20 changes: 18 additions & 2 deletions crates/cli/src/commands/env/action/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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?;
Expand Down
18 changes: 13 additions & 5 deletions crates/cli/src/commands/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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?;
Expand Down
18 changes: 12 additions & 6 deletions crates/cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?);
Expand Down
4 changes: 2 additions & 2 deletions crates/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand Down
16 changes: 10 additions & 6 deletions crates/common/src/action_models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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>,
Expand Down Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion crates/common/src/key_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions crates/common/src/rpc/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
6 changes: 3 additions & 3 deletions crates/controlplane/src/cannon/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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) => {
Expand All @@ -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;
}
}
Expand Down
Loading
Loading