BM-282: Shift error logs to warns (github#98)

This PR adjusts certain classes of errors that were non-fatal /
non-critical to `warn` logs. It does this by moving a bunch of Anyhow
errors to classified errors to allow for best effort classification of
what errors should be marked as `warn`

closes github#96 
closes BM-282

Author: mothran

crates/api/src/lib.rs

@@ -140,7 +140,11 @@ impl IntoResponse for AppError {
             Self::ReceiptMissing(_) | Self::JournalMissing(_) => StatusCode::NOT_FOUND,
             Self::InternalErr(_) | Self::DbError(_) => StatusCode::INTERNAL_SERVER_ERROR,
         };
-        tracing::error!("api error, code {code}: {self:?}");
+
+        match self {
+            Self::ImgAlreadyExists(_) => tracing::warn!("api warn, code: {code}, {self:?}"),
+            _ => tracing::error!("api error, code {code}: {self:?}"),
+        }
 
         (code, Json(ErrMsg { r#type: self.type_str(), msg: self.to_string() })).into_response()
     }

crates/boundless-market/src/contracts/proof_market.rs

@@ -26,14 +26,22 @@ use super::{
 pub enum MarketError {
     #[error("Transaction error: {0}")]
     TxnError(#[from] TxnErr),
+
     #[error("Request is not fulfilled {0}")]
     RequestNotFulfilled(U256),
+
     #[error("Request has expired {0}")]
     RequestHasExpired(U256),
+
     #[error("Proof not found {0}")]
     ProofNotFound(U256),
+
+    #[error("Lockin reverted, possibly outbid: txn_hash: {0}")]
+    LockRevert(B256),
+
     #[error("Market error: {0}")]
     Error(#[from] anyhow::Error),
+
     #[error("Timeout: {0}")]
     TimeoutReached(U256),
 }
@@ -252,10 +260,7 @@ where
 
         if !receipt.status() {
             // TODO: Get + print revertReason
-            return Err(MarketError::Error(anyhow!(
-                "LockinRequest failed [{}], possibly outbid",
-                receipt.transaction_hash
-            )));
+            return Err(MarketError::LockRevert(receipt.transaction_hash));
         }
 
         tracing::info!("Registered request {:x}: {}", request.id, receipt.transaction_hash);

crates/broker/src/lib.rs

@@ -92,7 +92,7 @@ pub struct Args {
 }
 
 /// Status of a order as it moves through the lifecycle
-#[derive(Clone, sqlx::Type, Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, Copy, sqlx::Type, Debug, PartialEq, Serialize, Deserialize)]
 enum OrderStatus {
     /// New order found on chain, waiting pricing analysis
     New,

crates/broker/src/market_monitor.rs

@@ -17,6 +17,7 @@ use boundless_market::contracts::{proof_market::ProofMarketService, IProofMarket
 use futures_util::StreamExt;
 
 use crate::{
+    db::DbError,
     task::{RetryRes, RetryTask, SupervisorErr},
     DbObj, Order,
 };
@@ -178,7 +179,20 @@ where
                             )
                             .await
                         {
-                            tracing::error!("Failed to add new order into DB: {err:?}");
+                            match err {
+                                DbError::SqlErr(sqlx::Error::Database(db_err)) => {
+                                    if db_err.is_unique_violation() {
+                                        tracing::warn!("Duplicate order detected: {db_err:?}");
+                                    } else {
+                                        tracing::error!(
+                                            "Failed to add new order into DB: {db_err:?}"
+                                        );
+                                    }
+                                }
+                                _ => {
+                                    tracing::error!("Failed to add new order into DB: {err:?}");
+                                }
+                            }
                         }
                     }
                     Err(err) => {

crates/broker/src/order_monitor.rs

@@ -14,10 +14,29 @@ use alloy::{
     providers::{Provider, WalletProvider},
     transports::Transport,
 };
-use anyhow::{bail, Context, Result};
-use boundless_market::contracts::{proof_market::ProofMarketService, ProofStatus};
+use anyhow::{Context, Result};
+use boundless_market::contracts::{
+    proof_market::{MarketError, ProofMarketService},
+    ProofStatus,
+};
 use std::sync::Arc;
 use std::time::Duration;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum LockOrderErr {
+    #[error("Failed to fetch / push image: {0}")]
+    OrderLockedInBlock(MarketError),
+
+    #[error("Invalid order status for locking: {0:?}")]
+    InvalidStatus(OrderStatus),
+
+    #[error("Order already locked")]
+    AlreadyLocked,
+
+    #[error("Other: {0}")]
+    OtherErr(#[from] anyhow::Error),
+}
 
 #[derive(Clone)]
 pub struct OrderMonitor<T, P> {
@@ -57,9 +76,9 @@ where
         Ok(Self { db, provider, block_time, config, market })
     }
 
-    async fn lock_order(&self, order_id: U256, order: &Order) -> Result<()> {
+    async fn lock_order(&self, order_id: U256, order: &Order) -> Result<(), LockOrderErr> {
         if order.status != OrderStatus::Locking {
-            bail!("Invalid order status for locking: {:?}", order.status);
+            return Err(LockOrderErr::InvalidStatus(order.status));
         }
 
         let order_status =
@@ -68,7 +87,7 @@ where
             tracing::warn!("Order {order_id:x} not open: {order_status:?}, skipping");
             // TODO: fetch some chain data to find out who / and for how much the order
             // was locked in at
-            bail!("Order already locked");
+            return Err(LockOrderErr::AlreadyLocked);
         }
 
         let conf_priority_gas = {
@@ -83,7 +102,8 @@ where
         let lock_block = self
             .market
             .lockin_request(&order.request, &order.client_sig, conf_priority_gas)
-            .await?;
+            .await
+            .map_err(LockOrderErr::OrderLockedInBlock)?;
 
         let lock_price = self
             .market
@@ -104,8 +124,16 @@ where
         for (order_id, order) in orders.iter() {
             match self.lock_order(*order_id, order).await {
                 Ok(_) => tracing::info!("Locked order: {order_id:x}"),
-                Err(err) => {
-                    tracing::error!("Failed to lock order: {order_id:x} {err}");
+                Err(ref err) => {
+                    match err {
+                        LockOrderErr::OtherErr(err) => {
+                            tracing::error!("Failed to lock order: {order_id:x} {err:?}");
+                        }
+                        // Only warn on known / classified errors
+                        _ => {
+                            tracing::warn!("Soft failed to lock order: {order_id:x} {err:?}");
+                        }
+                    }
                     if let Err(err) = self.db.set_order_failure(*order_id, format!("{err:?}")).await
                     {
                         tracing::error!(

crates/broker/src/order_picker.rs

@@ -15,11 +15,27 @@ use alloy::{
 };
 use anyhow::{Context, Result};
 use boundless_market::contracts::proof_market::ProofMarketService;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum PriceOrderErr {
+    #[error("Failed to fetch / push input: {0}")]
+    FetchInputErr(anyhow::Error),
+
+    #[error("Failed to fetch / push image: {0}")]
+    FetchImageErr(anyhow::Error),
+
+    #[error("Guest execution faulted: {0}")]
+    GuestPanic(String),
+
+    #[error("Other: {0}")]
+    OtherErr(#[from] anyhow::Error),
+}
 
 use crate::{
     config::ConfigLock,
     db::DbObj,
-    provers::ProverObj,
+    provers::{ProverError, ProverObj},
     task::{RetryRes, RetryTask, SupervisorErr},
     Order,
 };
@@ -55,7 +71,7 @@ where
         Self { db, config, prover, block_time, provider, market }
     }
 
-    async fn price_order(&self, order_id: U256, order: &Order) -> Result<()> {
+    async fn price_order(&self, order_id: U256, order: &Order) -> Result<(), PriceOrderErr> {
         tracing::debug!("Processing order {order_id:x}: {order:?}");
 
         let (min_deadline, allowed_addresses_opt) = {
@@ -154,11 +170,11 @@ where
         // TODO: Move URI handling like this into the prover impls
         let image_id = crate::upload_image_uri(&self.prover, order, max_size, fetch_retries)
             .await
-            .context("Failed to fetch and upload image_id")?;
+            .map_err(PriceOrderErr::FetchImageErr)?;
 
         let input_id = crate::upload_input_uri(&self.prover, order, max_size, fetch_retries)
             .await
-            .context("Failed to fetch and upload input_id")?;
+            .map_err(PriceOrderErr::FetchInputErr)?;
 
         // Record the image/input IDs for proving stage
         self.db
@@ -200,7 +216,18 @@ where
                 /* TODO assumptions */ Some(exec_limit * 1024 * 1024),
             )
             .await
-            .context("Preflight failed")?;
+            .map_err(|err| match err {
+                ProverError::ProvingFailed(ref err_msg) => {
+                    // TODO: Get enum'd errors from the SDK to prevent str
+                    // checks
+                    if err_msg.contains("GuestPanic") {
+                        PriceOrderErr::GuestPanic(err_msg.clone())
+                    } else {
+                        PriceOrderErr::OtherErr(err.into())
+                    }
+                }
+                _ => PriceOrderErr::OtherErr(err.into()),
+            })?;
 
         // TODO: this only checks that we could prove this at peak_khz, not if the cluster currently
         // can absorb that proving load, we need to cordinate this check with parallel
@@ -356,7 +383,17 @@ where
                                 .set_order_failure(order_id, err.to_string())
                                 .await
                                 .expect("Failed to set order failure");
-                            tracing::error!("pricing order failed: {order_id:x} {err:?}");
+                            match err {
+                                PriceOrderErr::OtherErr(err) => {
+                                    tracing::error!("Pricing order failed: {order_id:x} {err:?}");
+                                }
+                                // Only warn on known / classified errors
+                                _ => {
+                                    tracing::warn!(
+                                        "Pricing order soft failed: {order_id:x} {err:?}"
+                                    );
+                                }
+                            }
                         }
                     });
                 }

crates/broker/src/task.rs

@@ -40,7 +40,7 @@ pub async fn supervisor(count: usize, task: Arc<impl RetryTask>) -> AnyhowRes<()
                 Ok(_) => tracing::debug!("Task exited cleanly"),
                 Err(err) => match err {
                     SupervisorErr::Recover(err) => {
-                        tracing::error!(
+                        tracing::warn!(
                             "Recoverable failure detected: {err:?}, spawning replacement"
                         );
                         tasks.spawn(task.spawn());