From 5909f85b02b06c9c61c2cca9c92300c17145259f Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 26 Dec 2024 11:26:27 -0300 Subject: [PATCH 01/10] feat: add error for tx prover proxy chore: fix section headers --- Cargo.lock | 1 + bin/tx-prover/Cargo.toml | 1 + bin/tx-prover/src/commands/proxy.rs | 7 ++++--- bin/tx-prover/src/error.rs | 23 +++++++++++++++++++++++ bin/tx-prover/src/lib.rs | 2 ++ bin/tx-prover/src/proxy/mod.rs | 11 ++++++----- bin/tx-prover/src/proxy/worker.rs | 3 ++- bin/tx-prover/src/utils.rs | 12 +++++++----- 8 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 bin/tx-prover/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index ae5a66c47..77d214ea7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2038,6 +2038,7 @@ dependencies = [ "reqwest", "serde", "serde_qs", + "thiserror 2.0.3", "tokio", "tokio-stream", "toml", diff --git a/bin/tx-prover/Cargo.toml b/bin/tx-prover/Cargo.toml index 174b3c8eb..1a02075bb 100644 --- a/bin/tx-prover/Cargo.toml +++ b/bin/tx-prover/Cargo.toml @@ -61,6 +61,7 @@ serde_qs = { version = "0.13" } tokio = { version = "1.38", optional = true, features = ["full"] } tokio-stream = { version = "0.1", optional = true, features = [ "net" ]} toml = { version = "0.8" } +thiserror = { workspace = true } tonic-health = { version = "0.12" } tonic-web = { version = "0.12", optional = true } tracing = { version = "0.1", optional = true } diff --git a/bin/tx-prover/src/commands/proxy.rs b/bin/tx-prover/src/commands/proxy.rs index b0a9edd31..76a6cc7a3 100644 --- a/bin/tx-prover/src/commands/proxy.rs +++ b/bin/tx-prover/src/commands/proxy.rs @@ -1,4 +1,5 @@ use clap::Parser; +use miden_tx_prover::error::TxProverProxyError; use pingora::{ apps::HttpServerOptions, lb::Backend, @@ -40,8 +41,8 @@ impl StartProxy { let workers = self .workers .iter() - .map(|worker| Backend::new(worker).map_err(|err| err.to_string())) - .collect::, String>>()?; + .map(|worker| Backend::new(worker).map_err(TxProverProxyError::BackendCreationFailed)) + .collect::, TxProverProxyError>>()?; if workers.is_empty() { warn!("Starting the proxy without any workers"); @@ -58,7 +59,7 @@ impl StartProxy { let proxy_host = proxy_config.host; let proxy_port = proxy_config.port.to_string(); lb.add_tcp(format!("{}:{}", proxy_host, proxy_port).as_str()); - let logic = lb.app_logic_mut().ok_or("Failed to get app logic")?; + let logic = lb.app_logic_mut().ok_or(TxProverProxyError::AppLogicNotFound)?; let mut http_server_options = HttpServerOptions::default(); // Enable HTTP/2 for plaintext diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs new file mode 100644 index 000000000..05a3fc220 --- /dev/null +++ b/bin/tx-prover/src/error.rs @@ -0,0 +1,23 @@ +use axum::http::uri::InvalidUri; +use thiserror::Error; + +// TX PROVER ERROR +// ================================================================================================ + +#[derive(Debug, Error)] +pub enum TxProverProxyError { + #[error("invalid uri error")] + InvalidURI(#[source] InvalidUri), + #[error("failed to connect to worker")] + ConnectionFailed(#[source] tonic::transport::Error), + #[error("failed to create backend for worker")] + BackendCreationFailed(#[source] Box), + #[error("app logic not found")] + AppLogicNotFound, +} + +impl From for String { + fn from(err: TxProverProxyError) -> Self { + err.to_string() + } +} diff --git a/bin/tx-prover/src/lib.rs b/bin/tx-prover/src/lib.rs index f3140c422..e955b2cb5 100644 --- a/bin/tx-prover/src/lib.rs +++ b/bin/tx-prover/src/lib.rs @@ -4,6 +4,8 @@ use alloc::string::String; pub mod generated; +#[cfg(not(target_arch = "wasm32"))] +pub mod error; #[cfg(feature = "async")] mod prover; #[cfg(feature = "async")] diff --git a/bin/tx-prover/src/proxy/mod.rs b/bin/tx-prover/src/proxy/mod.rs index 49ec0d079..feacb6a1e 100644 --- a/bin/tx-prover/src/proxy/mod.rs +++ b/bin/tx-prover/src/proxy/mod.rs @@ -2,6 +2,7 @@ use std::{collections::VecDeque, future::Future, pin::Pin, sync::Arc, time::Dura use async_trait::async_trait; use bytes::Bytes; +use miden_tx_prover::error::TxProverProxyError; use once_cell::sync::Lazy; use pingora::{ http::ResponseHeader, @@ -36,7 +37,7 @@ mod worker; /// Localhost address const LOCALHOST_ADDR: &str = "127.0.0.1"; -// LOAD BALANCER +// LOAD BALANCER STATE // ================================================================================================ /// Load balancer that uses a round robin strategy @@ -58,7 +59,7 @@ impl LoadBalancerState { pub async fn new( initial_workers: Vec, config: &ProxyConfig, - ) -> core::result::Result { + ) -> core::result::Result { let mut workers: Vec = Vec::with_capacity(initial_workers.len()); let connection_timeout = Duration::from_secs(config.connection_timeout_secs); @@ -120,7 +121,7 @@ impl LoadBalancerState { pub async fn update_workers( &self, update_workers: UpdateWorkers, - ) -> std::result::Result<(), String> { + ) -> std::result::Result<(), TxProverProxyError> { let mut workers = self.workers.write().await; info!("Current workers: {:?}", workers); @@ -129,7 +130,7 @@ impl LoadBalancerState { .iter() .map(|worker| Backend::new(worker)) .collect::, _>>() - .map_err(|err| format!("Failed to create backend: {}", err))?; + .map_err(TxProverProxyError::BackendCreationFailed)?; let mut native_workers = Vec::new(); @@ -326,7 +327,7 @@ impl RequestContext { } } -// LOAD BALANCER WRAPPER +// LOAD BALANCER // ================================================================================================ /// Wrapper around the load balancer that implements the ProxyHttp trait diff --git a/bin/tx-prover/src/proxy/worker.rs b/bin/tx-prover/src/proxy/worker.rs index f42466df4..8919ac674 100644 --- a/bin/tx-prover/src/proxy/worker.rs +++ b/bin/tx-prover/src/proxy/worker.rs @@ -1,5 +1,6 @@ use std::time::Duration; +use miden_tx_prover::error::TxProverProxyError; use pingora::lb::Backend; use tonic::transport::Channel; use tonic_health::pb::{ @@ -28,7 +29,7 @@ impl Worker { worker: Backend, connection_timeout: Duration, total_timeout: Duration, - ) -> Result { + ) -> Result { let health_check_client = create_health_check_client(worker.addr.to_string(), connection_timeout, total_timeout) .await?; diff --git a/bin/tx-prover/src/utils.rs b/bin/tx-prover/src/utils.rs index 5e6c3e2bc..eeb275489 100644 --- a/bin/tx-prover/src/utils.rs +++ b/bin/tx-prover/src/utils.rs @@ -1,5 +1,6 @@ use std::time::Duration; +use miden_tx_prover::error::TxProverProxyError; use opentelemetry::{trace::TracerProvider as _, KeyValue}; use opentelemetry_sdk::{ runtime, @@ -163,13 +164,14 @@ pub async fn create_health_check_client( address: String, connection_timeout: Duration, total_timeout: Duration, -) -> Result, String> { - Channel::from_shared(format!("http://{}", address)) - .map_err(|err| format!("Invalid format for worker URI: {}", err))? +) -> Result, TxProverProxyError> { + let channel = Channel::from_shared(format!("http://{}", address)) + .map_err(TxProverProxyError::InvalidURI)? .connect_timeout(connection_timeout) .timeout(total_timeout) .connect() .await - .map(HealthClient::new) - .map_err(|err| format!("Failed to create health check client for worker: {}", err)) + .map_err(TxProverProxyError::ConnectionFailed)?; + + Ok(HealthClient::new(channel)) } From 23474d4201d4ded0467924dbe3516bfafccccbac Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Mon, 30 Dec 2024 12:42:33 -0300 Subject: [PATCH 02/10] review: remove 'error' from invalid uri error --- bin/tx-prover/src/error.rs | 2 +- bin/tx-prover/src/lib.rs | 2 -- bin/tx-prover/src/main.rs | 1 + 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs index 05a3fc220..71ccc9e4b 100644 --- a/bin/tx-prover/src/error.rs +++ b/bin/tx-prover/src/error.rs @@ -6,7 +6,7 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum TxProverProxyError { - #[error("invalid uri error")] + #[error("invalid uri")] InvalidURI(#[source] InvalidUri), #[error("failed to connect to worker")] ConnectionFailed(#[source] tonic::transport::Error), diff --git a/bin/tx-prover/src/lib.rs b/bin/tx-prover/src/lib.rs index e955b2cb5..f3140c422 100644 --- a/bin/tx-prover/src/lib.rs +++ b/bin/tx-prover/src/lib.rs @@ -4,8 +4,6 @@ use alloc::string::String; pub mod generated; -#[cfg(not(target_arch = "wasm32"))] -pub mod error; #[cfg(feature = "async")] mod prover; #[cfg(feature = "async")] diff --git a/bin/tx-prover/src/main.rs b/bin/tx-prover/src/main.rs index 176a353cf..0a7b1f968 100644 --- a/bin/tx-prover/src/main.rs +++ b/bin/tx-prover/src/main.rs @@ -1,5 +1,6 @@ pub mod api; pub mod commands; +pub mod error; mod proxy; mod utils; use commands::Cli; From 9887d379d83a80fea8cc0caf3c8a956092b230b9 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Mon, 30 Dec 2024 12:44:01 -0300 Subject: [PATCH 03/10] review: move mod export from lib to main --- bin/tx-prover/src/commands/proxy.rs | 2 +- bin/tx-prover/src/proxy/mod.rs | 2 +- bin/tx-prover/src/proxy/worker.rs | 3 +-- bin/tx-prover/src/utils.rs | 3 ++- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/tx-prover/src/commands/proxy.rs b/bin/tx-prover/src/commands/proxy.rs index 76a6cc7a3..16e126311 100644 --- a/bin/tx-prover/src/commands/proxy.rs +++ b/bin/tx-prover/src/commands/proxy.rs @@ -1,5 +1,4 @@ use clap::Parser; -use miden_tx_prover::error::TxProverProxyError; use pingora::{ apps::HttpServerOptions, lb::Backend, @@ -10,6 +9,7 @@ use pingora_proxy::http_proxy_service; use tracing::warn; use crate::{ + error::TxProverProxyError, proxy::{LoadBalancer, LoadBalancerState}, utils::MIDEN_TX_PROVER, }; diff --git a/bin/tx-prover/src/proxy/mod.rs b/bin/tx-prover/src/proxy/mod.rs index feacb6a1e..12c877083 100644 --- a/bin/tx-prover/src/proxy/mod.rs +++ b/bin/tx-prover/src/proxy/mod.rs @@ -2,7 +2,6 @@ use std::{collections::VecDeque, future::Future, pin::Pin, sync::Arc, time::Dura use async_trait::async_trait; use bytes::Bytes; -use miden_tx_prover::error::TxProverProxyError; use once_cell::sync::Lazy; use pingora::{ http::ResponseHeader, @@ -26,6 +25,7 @@ use crate::{ update_workers::{Action, UpdateWorkers}, ProxyConfig, }, + error::TxProverProxyError, utils::{ create_queue_full_response, create_response_with_error_message, create_too_many_requests_response, create_workers_updated_response, MIDEN_TX_PROVER, diff --git a/bin/tx-prover/src/proxy/worker.rs b/bin/tx-prover/src/proxy/worker.rs index 8919ac674..afae335a4 100644 --- a/bin/tx-prover/src/proxy/worker.rs +++ b/bin/tx-prover/src/proxy/worker.rs @@ -1,6 +1,5 @@ use std::time::Duration; -use miden_tx_prover::error::TxProverProxyError; use pingora::lb::Backend; use tonic::transport::Channel; use tonic_health::pb::{ @@ -8,7 +7,7 @@ use tonic_health::pb::{ }; use tracing::error; -use crate::utils::create_health_check_client; +use crate::{error::TxProverProxyError, utils::create_health_check_client}; // WORKER // ================================================================================================ diff --git a/bin/tx-prover/src/utils.rs b/bin/tx-prover/src/utils.rs index eeb275489..99b05fe6f 100644 --- a/bin/tx-prover/src/utils.rs +++ b/bin/tx-prover/src/utils.rs @@ -1,6 +1,5 @@ use std::time::Duration; -use miden_tx_prover::error::TxProverProxyError; use opentelemetry::{trace::TracerProvider as _, KeyValue}; use opentelemetry_sdk::{ runtime, @@ -17,6 +16,8 @@ use tonic::transport::Channel; use tonic_health::pb::health_client::HealthClient; use tracing_subscriber::{layer::SubscriberExt, Registry}; +use crate::error::TxProverProxyError; + pub const MIDEN_TX_PROVER: &str = "miden-tx-prover"; const RESOURCE_EXHAUSTED_CODE: u16 = 8; From 09d3defb00ec8dc0c01bf3482f6a1ca5689be05f Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Mon, 30 Dec 2024 12:44:47 -0300 Subject: [PATCH 04/10] review: rename to TxProverServiceError --- bin/tx-prover/src/commands/proxy.rs | 8 ++++---- bin/tx-prover/src/error.rs | 6 +++--- bin/tx-prover/src/proxy/mod.rs | 8 ++++---- bin/tx-prover/src/proxy/worker.rs | 4 ++-- bin/tx-prover/src/utils.rs | 8 ++++---- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/bin/tx-prover/src/commands/proxy.rs b/bin/tx-prover/src/commands/proxy.rs index 16e126311..8c06396eb 100644 --- a/bin/tx-prover/src/commands/proxy.rs +++ b/bin/tx-prover/src/commands/proxy.rs @@ -9,7 +9,7 @@ use pingora_proxy::http_proxy_service; use tracing::warn; use crate::{ - error::TxProverProxyError, + error::TxProverServiceError, proxy::{LoadBalancer, LoadBalancerState}, utils::MIDEN_TX_PROVER, }; @@ -41,8 +41,8 @@ impl StartProxy { let workers = self .workers .iter() - .map(|worker| Backend::new(worker).map_err(TxProverProxyError::BackendCreationFailed)) - .collect::, TxProverProxyError>>()?; + .map(|worker| Backend::new(worker).map_err(TxProverServiceError::BackendCreationFailed)) + .collect::, TxProverServiceError>>()?; if workers.is_empty() { warn!("Starting the proxy without any workers"); @@ -59,7 +59,7 @@ impl StartProxy { let proxy_host = proxy_config.host; let proxy_port = proxy_config.port.to_string(); lb.add_tcp(format!("{}:{}", proxy_host, proxy_port).as_str()); - let logic = lb.app_logic_mut().ok_or(TxProverProxyError::AppLogicNotFound)?; + let logic = lb.app_logic_mut().ok_or(TxProverServiceError::AppLogicNotFound)?; let mut http_server_options = HttpServerOptions::default(); // Enable HTTP/2 for plaintext diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs index 71ccc9e4b..537036773 100644 --- a/bin/tx-prover/src/error.rs +++ b/bin/tx-prover/src/error.rs @@ -5,7 +5,7 @@ use thiserror::Error; // ================================================================================================ #[derive(Debug, Error)] -pub enum TxProverProxyError { +pub enum TxProverServiceError { #[error("invalid uri")] InvalidURI(#[source] InvalidUri), #[error("failed to connect to worker")] @@ -16,8 +16,8 @@ pub enum TxProverProxyError { AppLogicNotFound, } -impl From for String { - fn from(err: TxProverProxyError) -> Self { +impl From for String { + fn from(err: TxProverServiceError) -> Self { err.to_string() } } diff --git a/bin/tx-prover/src/proxy/mod.rs b/bin/tx-prover/src/proxy/mod.rs index 12c877083..9a2ab66df 100644 --- a/bin/tx-prover/src/proxy/mod.rs +++ b/bin/tx-prover/src/proxy/mod.rs @@ -25,7 +25,7 @@ use crate::{ update_workers::{Action, UpdateWorkers}, ProxyConfig, }, - error::TxProverProxyError, + error::TxProverServiceError, utils::{ create_queue_full_response, create_response_with_error_message, create_too_many_requests_response, create_workers_updated_response, MIDEN_TX_PROVER, @@ -59,7 +59,7 @@ impl LoadBalancerState { pub async fn new( initial_workers: Vec, config: &ProxyConfig, - ) -> core::result::Result { + ) -> core::result::Result { let mut workers: Vec = Vec::with_capacity(initial_workers.len()); let connection_timeout = Duration::from_secs(config.connection_timeout_secs); @@ -121,7 +121,7 @@ impl LoadBalancerState { pub async fn update_workers( &self, update_workers: UpdateWorkers, - ) -> std::result::Result<(), TxProverProxyError> { + ) -> std::result::Result<(), TxProverServiceError> { let mut workers = self.workers.write().await; info!("Current workers: {:?}", workers); @@ -130,7 +130,7 @@ impl LoadBalancerState { .iter() .map(|worker| Backend::new(worker)) .collect::, _>>() - .map_err(TxProverProxyError::BackendCreationFailed)?; + .map_err(TxProverServiceError::BackendCreationFailed)?; let mut native_workers = Vec::new(); diff --git a/bin/tx-prover/src/proxy/worker.rs b/bin/tx-prover/src/proxy/worker.rs index afae335a4..866dfaea8 100644 --- a/bin/tx-prover/src/proxy/worker.rs +++ b/bin/tx-prover/src/proxy/worker.rs @@ -7,7 +7,7 @@ use tonic_health::pb::{ }; use tracing::error; -use crate::{error::TxProverProxyError, utils::create_health_check_client}; +use crate::{error::TxProverServiceError, utils::create_health_check_client}; // WORKER // ================================================================================================ @@ -28,7 +28,7 @@ impl Worker { worker: Backend, connection_timeout: Duration, total_timeout: Duration, - ) -> Result { + ) -> Result { let health_check_client = create_health_check_client(worker.addr.to_string(), connection_timeout, total_timeout) .await?; diff --git a/bin/tx-prover/src/utils.rs b/bin/tx-prover/src/utils.rs index 99b05fe6f..445b2273d 100644 --- a/bin/tx-prover/src/utils.rs +++ b/bin/tx-prover/src/utils.rs @@ -16,7 +16,7 @@ use tonic::transport::Channel; use tonic_health::pb::health_client::HealthClient; use tracing_subscriber::{layer::SubscriberExt, Registry}; -use crate::error::TxProverProxyError; +use crate::error::TxProverServiceError; pub const MIDEN_TX_PROVER: &str = "miden-tx-prover"; @@ -165,14 +165,14 @@ pub async fn create_health_check_client( address: String, connection_timeout: Duration, total_timeout: Duration, -) -> Result, TxProverProxyError> { +) -> Result, TxProverServiceError> { let channel = Channel::from_shared(format!("http://{}", address)) - .map_err(TxProverProxyError::InvalidURI)? + .map_err(TxProverServiceError::InvalidURI)? .connect_timeout(connection_timeout) .timeout(total_timeout) .connect() .await - .map_err(TxProverProxyError::ConnectionFailed)?; + .map_err(TxProverServiceError::ConnectionFailed)?; Ok(HealthClient::new(channel)) } From c2a335575fcfcc78d890c473a021d4e675af9dba Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Mon, 30 Dec 2024 12:53:28 -0300 Subject: [PATCH 05/10] review: replace AppLogic error with a more generic Pingora error --- bin/tx-prover/src/commands/proxy.rs | 4 +++- bin/tx-prover/src/error.rs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/bin/tx-prover/src/commands/proxy.rs b/bin/tx-prover/src/commands/proxy.rs index 8c06396eb..55bf092be 100644 --- a/bin/tx-prover/src/commands/proxy.rs +++ b/bin/tx-prover/src/commands/proxy.rs @@ -59,7 +59,9 @@ impl StartProxy { let proxy_host = proxy_config.host; let proxy_port = proxy_config.port.to_string(); lb.add_tcp(format!("{}:{}", proxy_host, proxy_port).as_str()); - let logic = lb.app_logic_mut().ok_or(TxProverServiceError::AppLogicNotFound)?; + let logic = lb + .app_logic_mut() + .ok_or(TxProverServiceError::PingoraConfigFailed("app logic not found".to_string()))?; let mut http_server_options = HttpServerOptions::default(); // Enable HTTP/2 for plaintext diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs index 537036773..3deae13b3 100644 --- a/bin/tx-prover/src/error.rs +++ b/bin/tx-prover/src/error.rs @@ -12,8 +12,8 @@ pub enum TxProverServiceError { ConnectionFailed(#[source] tonic::transport::Error), #[error("failed to create backend for worker")] BackendCreationFailed(#[source] Box), - #[error("app logic not found")] - AppLogicNotFound, + #[error("failed to setup pingora")] + PingoraConfigFailed(String), } impl From for String { From c2b85520920bcfb4a562aeac54fe808e3d75ed10 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Mon, 30 Dec 2024 13:00:37 -0300 Subject: [PATCH 06/10] review: add error cases to docs --- bin/tx-prover/src/commands/proxy.rs | 6 ++++++ bin/tx-prover/src/proxy/mod.rs | 3 +++ bin/tx-prover/src/proxy/worker.rs | 5 +++++ bin/tx-prover/src/utils.rs | 4 +++- 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/bin/tx-prover/src/commands/proxy.rs b/bin/tx-prover/src/commands/proxy.rs index 55bf092be..a5e9098e1 100644 --- a/bin/tx-prover/src/commands/proxy.rs +++ b/bin/tx-prover/src/commands/proxy.rs @@ -31,6 +31,12 @@ impl StartProxy { /// /// This method will first read the config file to get the parameters for the proxy. It will /// then start a proxy with each worker passed as command argument as a backend. + /// + /// Errors: + /// - If the config file cannot be read. + /// - If the backend cannot be created. + /// - If the Pingora configuration fails. + /// - If the server cannot be started. #[tracing::instrument(target = MIDEN_TX_PROVER, name = "proxy:execute")] pub async fn execute(&self) -> Result<(), String> { let mut server = Server::new(Some(Opt::default())).map_err(|err| err.to_string())?; diff --git a/bin/tx-prover/src/proxy/mod.rs b/bin/tx-prover/src/proxy/mod.rs index 9a2ab66df..cbc9a0682 100644 --- a/bin/tx-prover/src/proxy/mod.rs +++ b/bin/tx-prover/src/proxy/mod.rs @@ -55,6 +55,9 @@ pub struct LoadBalancerState { impl LoadBalancerState { /// Create a new load balancer + /// + /// Errors: + /// - If the worker cannot be created. #[tracing::instrument(name = "proxy:new_load_balancer", skip(initial_workers))] pub async fn new( initial_workers: Vec, diff --git a/bin/tx-prover/src/proxy/worker.rs b/bin/tx-prover/src/proxy/worker.rs index 866dfaea8..59ef53b2e 100644 --- a/bin/tx-prover/src/proxy/worker.rs +++ b/bin/tx-prover/src/proxy/worker.rs @@ -24,6 +24,11 @@ pub struct Worker { } impl Worker { + /// Creates a new worker and a gRPC health check client for the given worker address. + /// + /// Errors: + /// - Returns [TxProverServiceError::InvalidURI] if the worker address is invalid. + /// - Returns [TxProverServiceError::ConnectionFailed] if the connection to the worker fails. pub async fn new( worker: Backend, connection_timeout: Duration, diff --git a/bin/tx-prover/src/utils.rs b/bin/tx-prover/src/utils.rs index 445b2273d..9feed9278 100644 --- a/bin/tx-prover/src/utils.rs +++ b/bin/tx-prover/src/utils.rs @@ -160,7 +160,9 @@ pub async fn create_response_with_error_message( /// Create a gRPC [HealthClient] for the given worker address. /// -/// It will panic if the worker URI is invalid. +/// Errors: +/// - [TxProverServiceError::InvalidURI] if the worker address is invalid. +/// - [TxProverServiceError::ConnectionFailed] if the connection to the worker fails. pub async fn create_health_check_client( address: String, connection_timeout: Duration, From c44fe68f5edb186521bd256530557715b34d2071 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 2 Jan 2025 11:34:10 -0300 Subject: [PATCH 07/10] review: change section separator to TX PROVER SERVICE ERROR --- bin/tx-prover/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs index 3deae13b3..fcb89298f 100644 --- a/bin/tx-prover/src/error.rs +++ b/bin/tx-prover/src/error.rs @@ -1,7 +1,7 @@ use axum::http::uri::InvalidUri; use thiserror::Error; -// TX PROVER ERROR +// TX PROVER SERVICE ERROR // ================================================================================================ #[derive(Debug, Error)] From 42296923a59db58c592f3223a363c22b2c708a33 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 2 Jan 2025 11:34:52 -0300 Subject: [PATCH 08/10] review: use # Errors instead of Errors: for docs --- bin/tx-prover/src/commands/proxy.rs | 11 ++++++----- bin/tx-prover/src/proxy/mod.rs | 5 +++-- bin/tx-prover/src/proxy/worker.rs | 2 +- bin/tx-prover/src/utils.rs | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/bin/tx-prover/src/commands/proxy.rs b/bin/tx-prover/src/commands/proxy.rs index a5e9098e1..12c06c87a 100644 --- a/bin/tx-prover/src/commands/proxy.rs +++ b/bin/tx-prover/src/commands/proxy.rs @@ -32,11 +32,12 @@ impl StartProxy { /// This method will first read the config file to get the parameters for the proxy. It will /// then start a proxy with each worker passed as command argument as a backend. /// - /// Errors: - /// - If the config file cannot be read. - /// - If the backend cannot be created. - /// - If the Pingora configuration fails. - /// - If the server cannot be started. + /// # Errors + /// Returns an error in the following cases: + /// - The config file cannot be read. + /// - The backend cannot be created. + /// - The Pingora configuration fails. + /// - The server cannot be started. #[tracing::instrument(target = MIDEN_TX_PROVER, name = "proxy:execute")] pub async fn execute(&self) -> Result<(), String> { let mut server = Server::new(Some(Opt::default())).map_err(|err| err.to_string())?; diff --git a/bin/tx-prover/src/proxy/mod.rs b/bin/tx-prover/src/proxy/mod.rs index cbc9a0682..73b654588 100644 --- a/bin/tx-prover/src/proxy/mod.rs +++ b/bin/tx-prover/src/proxy/mod.rs @@ -56,8 +56,9 @@ pub struct LoadBalancerState { impl LoadBalancerState { /// Create a new load balancer /// - /// Errors: - /// - If the worker cannot be created. + /// # Errors + /// Returns an error if: + /// - The worker cannot be created. #[tracing::instrument(name = "proxy:new_load_balancer", skip(initial_workers))] pub async fn new( initial_workers: Vec, diff --git a/bin/tx-prover/src/proxy/worker.rs b/bin/tx-prover/src/proxy/worker.rs index 59ef53b2e..0d8cf3e4b 100644 --- a/bin/tx-prover/src/proxy/worker.rs +++ b/bin/tx-prover/src/proxy/worker.rs @@ -26,7 +26,7 @@ pub struct Worker { impl Worker { /// Creates a new worker and a gRPC health check client for the given worker address. /// - /// Errors: + /// # Errors /// - Returns [TxProverServiceError::InvalidURI] if the worker address is invalid. /// - Returns [TxProverServiceError::ConnectionFailed] if the connection to the worker fails. pub async fn new( diff --git a/bin/tx-prover/src/utils.rs b/bin/tx-prover/src/utils.rs index 9feed9278..a6b69e14c 100644 --- a/bin/tx-prover/src/utils.rs +++ b/bin/tx-prover/src/utils.rs @@ -160,7 +160,7 @@ pub async fn create_response_with_error_message( /// Create a gRPC [HealthClient] for the given worker address. /// -/// Errors: +/// # Errors /// - [TxProverServiceError::InvalidURI] if the worker address is invalid. /// - [TxProverServiceError::ConnectionFailed] if the connection to the worker fails. pub async fn create_health_check_client( From afe1b00222a50c6ecf17143c782f8ee2451fc455 Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 2 Jan 2025 11:36:21 -0300 Subject: [PATCH 09/10] review: add string to error message --- bin/tx-prover/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs index fcb89298f..8acf3c015 100644 --- a/bin/tx-prover/src/error.rs +++ b/bin/tx-prover/src/error.rs @@ -12,7 +12,7 @@ pub enum TxProverServiceError { ConnectionFailed(#[source] tonic::transport::Error), #[error("failed to create backend for worker")] BackendCreationFailed(#[source] Box), - #[error("failed to setup pingora")] + #[error("failed to setup pingora: {0}")] PingoraConfigFailed(String), } From ba32c282a3d2aea2bb8328efa5264ea6b7a62feb Mon Sep 17 00:00:00 2001 From: SantiagoPittella Date: Thu, 2 Jan 2025 11:54:28 -0300 Subject: [PATCH 10/10] feat: add origin worker to errors --- bin/tx-prover/src/error.rs | 8 ++++---- bin/tx-prover/src/utils.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/tx-prover/src/error.rs b/bin/tx-prover/src/error.rs index 8acf3c015..d69136213 100644 --- a/bin/tx-prover/src/error.rs +++ b/bin/tx-prover/src/error.rs @@ -6,10 +6,10 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum TxProverServiceError { - #[error("invalid uri")] - InvalidURI(#[source] InvalidUri), - #[error("failed to connect to worker")] - ConnectionFailed(#[source] tonic::transport::Error), + #[error("invalid uri {1}")] + InvalidURI(#[source] InvalidUri, String), + #[error("failed to connect to worker {1}")] + ConnectionFailed(#[source] tonic::transport::Error, String), #[error("failed to create backend for worker")] BackendCreationFailed(#[source] Box), #[error("failed to setup pingora: {0}")] diff --git a/bin/tx-prover/src/utils.rs b/bin/tx-prover/src/utils.rs index a6b69e14c..6cfdc2ac8 100644 --- a/bin/tx-prover/src/utils.rs +++ b/bin/tx-prover/src/utils.rs @@ -169,12 +169,12 @@ pub async fn create_health_check_client( total_timeout: Duration, ) -> Result, TxProverServiceError> { let channel = Channel::from_shared(format!("http://{}", address)) - .map_err(TxProverServiceError::InvalidURI)? + .map_err(|err| TxProverServiceError::InvalidURI(err, address.clone()))? .connect_timeout(connection_timeout) .timeout(total_timeout) .connect() .await - .map_err(TxProverServiceError::ConnectionFailed)?; + .map_err(|err| TxProverServiceError::ConnectionFailed(err, address))?; Ok(HealthClient::new(channel)) }