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

feat(sozo): provider health check #2745

Merged
merged 8 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions bin/sozo/src/commands/migrate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use anyhow::{Context, Result};
use crate::commands::LOG_TARGET;
use anyhow::{anyhow, Context, Result};
use clap::Args;
use colored::*;
use dojo_utils::utils::health_check_provider;
use dojo_utils::{self, TxnConfig};
use dojo_world::contracts::WorldContract;
use dojo_world::services::IpfsService;
Expand All @@ -10,9 +12,10 @@ use sozo_ops::migration_ui::MigrationUi;
use sozo_scarbext::WorkspaceExt;
use starknet::core::utils::parse_cairo_short_string;
use starknet::providers::Provider;
use std::sync::Arc;
use tabled::settings::Style;
use tabled::{Table, Tabled};
use tracing::trace;
use tracing::{error, trace};

use super::options::account::AccountOptions;
use super::options::ipfs::IpfsOptions;
Expand Down Expand Up @@ -141,6 +144,12 @@ async fn print_banner(ws: &Workspace<'_>, starknet: &StarknetOptions) -> Result<
let profile_config = ws.load_profile_config()?;
let (provider, rpc_url) = starknet.provider(profile_config.env.as_ref())?;

let provider = Arc::new(provider);
if let Err(e) = health_check_provider(provider.clone()).await {
error!(target: LOG_TARGET,"Provider health check failed during sozo migrate.");
return Err(e);
}
let provider = Arc::try_unwrap(provider).map_err(|_| anyhow!("Failed to unwrap Arc"))?;
let chain_id = provider.chain_id().await?;
let chain_id =
Comment on lines +152 to 153
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider optimizing Arc usage, sensei!

The current implementation wraps and immediately unwraps the provider. Since the health check is the only async operation needing the Arc, we could optimize this.

Consider this more efficient approach:

-let provider = Arc::new(provider);
-if let Err(e) = health_check_provider(provider.clone()).await {
-    error!(target: LOG_TARGET,"Provider health check failed during sozo migrate.");
-    return Err(e);
-}
-let provider = Arc::try_unwrap(provider).map_err(|_| anyhow!("Failed to unwrap Arc"))?;
+if let Err(e) = health_check_provider(Arc::new(provider.clone())).await {
+    error!(target: LOG_TARGET,"Provider health check failed during sozo migrate.");
+    return Err(e);
+}

This approach:

  1. Avoids unnecessary Arc unwrapping
  2. Reduces potential for errors
  3. Maintains the same functionality

Committable suggestion skipped: line range outside the PR's diff.

parse_cairo_short_string(&chain_id).with_context(|| "Cannot parse chain_id as string")?;
Expand Down
8 changes: 6 additions & 2 deletions bin/sozo/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ use migrate::MigrateArgs;
use model::ModelArgs;
use test::TestArgs;

pub(crate) const LOG_TARGET: &str = "sozo::cli";

#[derive(Debug, Subcommand)]
pub enum Commands {
#[command(about = "Grant or revoke a contract permission to write to a resource")]
Expand All @@ -42,8 +44,10 @@ pub enum Commands {
Build(Box<BuildArgs>),
#[command(about = "Build and migrate the world every time a file changes")]
Dev(Box<DevArgs>),
#[command(about = "Run a migration, declaring and deploying contracts as necessary to update \
the world")]
#[command(
about = "Run a migration, declaring and deploying contracts as necessary to update \
the world"
)]
Migrate(Box<MigrateArgs>),
#[command(about = "Execute a system with the given calldata.")]
Execute(Box<ExecuteArgs>),
Expand Down
11 changes: 10 additions & 1 deletion bin/sozo/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use std::collections::HashMap;
use std::io::{self, Write};
use std::str::FromStr;
use std::sync::Arc;

use crate::commands::LOG_TARGET;
use anyhow::{anyhow, Context, Result};
use camino::Utf8PathBuf;
use colored::*;
use dojo_utils::utils::health_check_provider;
use dojo_world::config::ProfileConfig;
use dojo_world::contracts::ContractInfo;
use dojo_world::diff::WorldDiff;
Expand All @@ -19,7 +22,7 @@ use starknet::core::types::Felt;
use starknet::core::utils as snutils;
use starknet::providers::jsonrpc::HttpTransport;
use starknet::providers::{JsonRpcClient, Provider};
use tracing::trace;
use tracing::{error, trace};

use crate::commands::options::account::{AccountOptions, SozoAccount};
use crate::commands::options::starknet::StarknetOptions;
Expand Down Expand Up @@ -113,6 +116,12 @@ pub async fn get_world_diff_and_provider(
let world_address = get_world_address(&profile_config, &world, &world_local)?;

let (provider, rpc_url) = starknet.provider(env)?;
let provider = Arc::new(provider);
if let Err(e) = health_check_provider(provider.clone()).await {
error!(target: LOG_TARGET,"Provider health check failed during sozo inspect.");
return Err(e);
}
let provider = Arc::try_unwrap(provider).map_err(|_| anyhow!("Failed to unwrap Arc"))?;
trace!(?provider, "Provider initialized.");

let spec_version = provider.spec_version().await?;
Expand Down
1 change: 1 addition & 0 deletions crates/dojo/utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ pub mod env;
pub mod keystore;

pub mod signal;
pub mod utils;
22 changes: 22 additions & 0 deletions crates/dojo/utils/src/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use starknet::core::types::{BlockId, BlockTag};
use starknet::providers::Provider;
use tracing::trace;

pub async fn health_check_provider<P: Provider + Sync + std::fmt::Debug + 'static>(
provider: P,
) -> anyhow::Result<(), anyhow::Error> {
match provider.get_block_with_tx_hashes(BlockId::Tag(BlockTag::Latest)).await {
Ok(block) => {
trace!(
latest_block = ?block,
"Provider health check."
);
Ok(())
}
Err(_) => {
let error_info =
format!("Unhealthy provider {:?}, please check your configuration.", provider);
Err(anyhow::anyhow!(error_info))
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling with more actionable messages

The current error message indicates the provider is unhealthy but could be more helpful to users.

Consider providing more specific guidance:

     Err(_) => {
         let error_info =
-            format!("Unhealthy provider {:?}, please check your configuration.", provider);
+            format!(
+                "Provider {:?} is not responding. Please ensure:\n\
+                 1. The provider URL is correct in your configuration\n\
+                 2. The provider service is running and accessible\n\
+                 3. Your network connection is stable",
+                provider
+            );
         Err(anyhow::anyhow!(error_info))
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Err(_) => {
let error_info =
format!("Unhealthy provider {:?}, please check your configuration.", provider);
Err(anyhow::anyhow!(error_info))
}
}
Err(_) => {
let error_info =
format!(
"Provider {:?} is not responding. Please ensure:\n\
1. The provider URL is correct in your configuration\n\
2. The provider service is running and accessible\n\
3. Your network connection is stable",
provider
);
Err(anyhow::anyhow!(error_info))
}
}

}
1 change: 1 addition & 0 deletions crates/torii/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ chrono.workspace = true
crypto-bigint.workspace = true
data-url.workspace = true
dojo-types.workspace = true
dojo-utils.workspace = true
dojo-world.workspace = true
futures-channel = "0.3.0"
futures-util.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion crates/torii/core/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::time::Duration;

use anyhow::Result;
use bitflags::bitflags;
use dojo_utils::utils::health_check_provider;
use dojo_world::contracts::world::WorldContractReader;
use futures_util::future::{join_all, try_join_all};
use hashlink::LinkedHashMap;
Expand Down Expand Up @@ -45,7 +46,6 @@ use crate::processors::{
};
use crate::sql::{Cursors, Sql};
use crate::types::{Contract, ContractType};
use crate::utils::health_check_provider;

type EventProcessorMap<P> = HashMap<Felt, Vec<Box<dyn EventProcessor<P>>>>;

Expand Down
23 changes: 1 addition & 22 deletions crates/torii/core/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ use anyhow::Result;
use chrono::{DateTime, Utc};
use futures_util::TryStreamExt;
use ipfs_api_backend_hyper::{IpfsApi, IpfsClient, TryFromUri};
use starknet::core::types::{BlockId, BlockTag};
use starknet::providers::Provider;
use tokio_util::bytes::Bytes;
use tracing::{info, trace};
use tracing::info;

use crate::constants::{
IPFS_CLIENT_MAX_RETRY, IPFS_CLIENT_PASSWORD, IPFS_CLIENT_URL, IPFS_CLIENT_USERNAME,
Expand Down Expand Up @@ -48,25 +46,6 @@ pub async fn fetch_content_from_ipfs(cid: &str, mut retries: u8) -> Result<Bytes
)))
}

pub async fn health_check_provider<P: Provider + Sync + std::fmt::Debug + 'static>(
provider: P,
) -> Result<(), anyhow::Error> {
match provider.get_block_with_tx_hashes(BlockId::Tag(BlockTag::Latest)).await {
Ok(block) => {
trace!(
latest_block = ?block,
"Provider health check."
);
Ok(())
}
Err(_) => {
let error_info =
format!("Unhealthy provider {:?}, please check your configuration.", provider);
Err(anyhow::anyhow!(error_info))
}
}
}

// tests
#[cfg(test)]
mod tests {
Expand Down
Loading