From 2c76bf09c7a2a5281542308d32fb6cce79373d7c Mon Sep 17 00:00:00 2001 From: Dagur Valberg Johannsson Date: Wed, 21 Jan 2026 08:27:57 +0100 Subject: [PATCH] Fix incorrect 5XX responses for user input errors Return proper 4XX status codes for client errors to prevent load balancers from misinterpreting input validation failures as server errors. Add centralized ApiResult type and ApiErrorCode enum for consistent handling. --- src/rpc/apy/mod.rs | 28 +++++----- src/rpc/bcmr.rs | 54 +++++++++--------- src/rpc/candlesticks.rs | 50 ++++++++++------- src/rpc/contract.rs | 40 +++++--------- src/rpc/err.rs | 107 ++++++++++++++++++++++++++++++++++- src/rpc/oracle.rs | 74 ++++++++++++++----------- src/rpc/pool.rs | 81 ++++++++++++--------------- src/rpc/price.rs | 120 +++++++++++++++++++++++++++------------- src/rpc/tokens.rs | 83 +++++++++++++-------------- src/rpc/tvl.rs | 45 ++++----------- src/rpc/tx.rs | 22 ++++---- src/rpc/user.rs | 15 ++--- src/rpc/volume.rs | 29 +++------- 13 files changed, 424 insertions(+), 324 deletions(-) diff --git a/src/rpc/apy/mod.rs b/src/rpc/apy/mod.rs index 528dc42..20ee023 100644 --- a/src/rpc/apy/mod.rs +++ b/src/rpc/apy/mod.rs @@ -4,11 +4,12 @@ // A copy of the license can be found in the LICENSE file or at https://www.gnu.org/licenses/agpl-3.0.html use poolperiod::PoolPeriod; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use serde_json::Value; use crate::{ db::{cauldron::pool::get_pool_period_snapshot, DB}, + rpc::err::{bad_request, db_error, ApiErrorCode, ApiResult}, timeutil::time_now, }; use serde_json::json; @@ -59,40 +60,37 @@ pub fn aggregate_apy( start: Option, // default 30 days before end end: Option, // default now db: &State, -) -> Result, Custom> { +) -> ApiResult { let end = end.unwrap_or(time_now()); let start = start.unwrap_or(end - (3600 * 24 * 30)); // 30 days if end < start { - return Err(Custom( - Status::BadRequest, - "end time cannot be less than start time".to_string(), + return Err(bad_request( + ApiErrorCode::InvalidTimeRange, + "end time cannot be less than start time", )); } if start < 0 { - return Err(Custom( - Status::BadRequest, - "start time cannot be negative".to_string(), + return Err(bad_request( + ApiErrorCode::InvalidTimeRange, + "start time cannot be negative", )); } - let conn = db - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("DB error: {e}")))?; + let conn = db.cauldron_r.get().map_err(db_error)?; let pools: anyhow::Result> = get_pool_period_snapshot(&conn, token, pkh, start, end) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))? + .map_err(db_error)? .into_iter() .map(|(start, end)| PoolPeriod::new(start, end)) .collect(); - let pools = pools.map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let pools = pools.map_err(db_error)?; let pools_count = pools.len(); let apy = apyaggregator::APYAggregator::aggregate_apy(pools.into_iter(), Some(start as u64)) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + .map_err(db_error)?; Ok(Json(json!({ "apy": apy.to_string(), diff --git a/src/rpc/bcmr.rs b/src/rpc/bcmr.rs index d24a8c4..5eb28fd 100644 --- a/src/rpc/bcmr.rs +++ b/src/rpc/bcmr.rs @@ -6,18 +6,21 @@ use anyhow::Context; use bitcoin_hashes::hex::{FromHex, ToHex}; use bitcoincash::TokenID; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use serde_json::json; use serde_json::Value; use crate::db::bcmr::get_well_known_bcmr; use crate::db::{bcmr::get_token_bcmr, DB}; +use crate::rpc::err::{bad_request, db_error, ApiErrorCode, ApiResult}; /// Fetches BCMR data for token from on-chain registry. /// Status: Stable /// /// - category: Token ID or symbol /// +/// Returns `null` if no BCMR data is found for the token. +/// /// **Response Example:** /// /// ```json @@ -41,53 +44,52 @@ use crate::db::{bcmr::get_token_bcmr, DB}; /// } /// ``` #[get("/token/")] -pub fn token_bcmr(category: Option<&str>, db: &State) -> Result, Custom> { +pub fn token_bcmr(category: Option<&str>, db: &State) -> ApiResult { let token_id_hex = category .context("category missing") - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + .map_err(|e| bad_request(ApiErrorCode::MissingCategory, &format!("Error: {e}")))?; // validate input by parsing it into TokenID - let token_id = TokenID::from_hex(token_id_hex) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; - let conn = db - .bcmr_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; - let bcmr = get_token_bcmr(&conn, &token_id.to_hex()) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let token_id = TokenID::from_hex(token_id_hex).map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token ID: {e}"), + ) + })?; + let conn = db.bcmr_r.get().map_err(db_error)?; + let bcmr = get_token_bcmr(&conn, &token_id.to_hex()).map_err(db_error)?; + // Return null (with 200 OK) if no BCMR found - this is not an error condition Ok(Json(json!(bcmr))) } /// Fetches BCMR data for token from all registries (including OTR). /// /// Return format is same as `/token/` route; except it returns an array of BCMR entries. +/// Returns an empty array if no BCMR data is found. #[get("/token//all")] -pub fn token_bcmr_all( - category: Option<&str>, - db: &State, -) -> Result, Custom> { +pub fn token_bcmr_all(category: Option<&str>, db: &State) -> ApiResult { let token_id_hex = category .context("category missing") - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + .map_err(|e| bad_request(ApiErrorCode::MissingCategory, &format!("Error: {e}")))?; // validate input by parsing it into TokenID - let token_id = TokenID::from_hex(token_id_hex) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; - let conn = db - .bcmr_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let token_id = TokenID::from_hex(token_id_hex).map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token ID: {e}"), + ) + })?; + let conn = db.bcmr_r.get().map_err(db_error)?; - let onchain_bcmr = get_token_bcmr(&conn, &token_id.to_hex()) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let onchain_bcmr = get_token_bcmr(&conn, &token_id.to_hex()).map_err(db_error)?; - let mut bcmr_entries = get_well_known_bcmr(&conn, &token_id.to_hex()) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let mut bcmr_entries = get_well_known_bcmr(&conn, &token_id.to_hex()).map_err(db_error)?; if let Some(bcmr) = onchain_bcmr { bcmr_entries.push(bcmr) } + // Returns empty array if no BCMR found - this is not an error condition Ok(Json(json!(bcmr_entries))) } diff --git a/src/rpc/candlesticks.rs b/src/rpc/candlesticks.rs index ff321c3..ade69a0 100644 --- a/src/rpc/candlesticks.rs +++ b/src/rpc/candlesticks.rs @@ -4,9 +4,10 @@ // A copy of the license can be found in the LICENSE file or at https://www.gnu.org/licenses/agpl-3.0.html use crate::db::DB; +use crate::rpc::err::{bad_request, db_error, ApiErrorCode, ApiResult}; use crate::timeutil::time_now; use anyhow::{bail, Result}; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use rusqlite::{params, Connection}; use serde::Serialize; use serde_json::json; @@ -277,15 +278,15 @@ pub fn price_candlesticks( end: Option, stepsize: Option, conn: &State, -) -> Result, Custom> { +) -> ApiResult { let current_timestamp = time_now(); // Validate that the provided end timestamp is not in the future. if let Some(end_ts) = end { if end_ts > current_timestamp { - return Err(Custom( - Status::BadRequest, - "End timestamp cannot be in the future".to_string(), + return Err(bad_request( + ApiErrorCode::FutureTimestamp, + "End timestamp cannot be in the future", )); } } @@ -296,17 +297,17 @@ pub fn price_candlesticks( let effective_stepsize = stepsize.unwrap_or(3600); if effective_stepsize <= 0 { - return Err(Custom( - Status::BadRequest, - "stepsize must be > 0".to_string(), + return Err(bad_request( + ApiErrorCode::InvalidStepsize, + "stepsize must be > 0", )); } // Validate that start is before end. if effective_start >= effective_end { - return Err(Custom( - Status::BadRequest, - "Start timestamp must be before end timestamp".to_string(), + return Err(bad_request( + ApiErrorCode::InvalidTimeRange, + "Start timestamp must be before end timestamp", )); } @@ -314,16 +315,13 @@ pub fn price_candlesticks( const MAX_INTERVALS: i64 = 10000; let total_intervals = (effective_end - effective_start) / effective_stepsize; if total_intervals > MAX_INTERVALS { - return Err(Custom( - Status::BadRequest, - format!("Too many intervals ({total_intervals} > {MAX_INTERVALS})"), + return Err(bad_request( + ApiErrorCode::TooManyIntervals, + &format!("Too many intervals ({total_intervals} > {MAX_INTERVALS})"), )); } - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let db = conn.cauldron_r.get().map_err(db_error)?; let candlesticks = candlesticks( &db, @@ -332,7 +330,7 @@ pub fn price_candlesticks( effective_stepsize, token, ) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + .map_err(|e| bad_request(ApiErrorCode::InvalidParameters, &e.to_string()))?; let candlesticks_json: Vec = candlesticks .iter() @@ -544,7 +542,12 @@ mod tests { assert_eq!(response.status(), Status::BadRequest); let body = response.into_string().unwrap_or_default(); - assert!(body.contains("End timestamp cannot be in the future")); + let json: serde_json::Value = serde_json::from_str(&body).expect("Response should be JSON"); + assert_eq!(json["error"]["code"], "FUTURE_TIMESTAMP"); + assert!(json["error"]["message"] + .as_str() + .unwrap() + .contains("future")); } #[test] @@ -568,7 +571,12 @@ mod tests { assert_eq!(response.status(), Status::BadRequest); let body = response.into_string().unwrap_or_default(); - assert!(body.contains("Start timestamp must be before end timestamp")); + let json: serde_json::Value = serde_json::from_str(&body).expect("Response should be JSON"); + assert_eq!(json["error"]["code"], "INVALID_TIME_RANGE"); + assert!(json["error"]["message"] + .as_str() + .unwrap() + .contains("before end")); } #[test] diff --git a/src/rpc/contract.rs b/src/rpc/contract.rs index b9b3ead..e1d46ad 100644 --- a/src/rpc/contract.rs +++ b/src/rpc/contract.rs @@ -5,12 +5,16 @@ use anyhow::Result; use rayon::prelude::*; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use rusqlite::Connection; use serde::Serialize; use serde_json::{json, Value}; -use crate::{db::DB, timeutil::time_now}; +use crate::{ + db::DB, + rpc::err::{bad_request, db_error, ApiErrorCode, ApiResult}, + timeutil::time_now, +}; #[derive(Serialize)] pub struct ContractCount { @@ -78,13 +82,9 @@ fn db_contract_count_by_token(db: &Connection, token_id: &str) -> Result) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; - let count = db_contract_count_all(&db) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; +pub fn contract_count_all(conn: &State) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; + let count = db_contract_count_all(&db).map_err(db_error)?; Ok(Json(json!(count))) } @@ -98,13 +98,9 @@ pub fn contract_count_all(conn: &State) -> Result, Custom")] -pub fn contract_count_token(token: &str, conn: &State) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; - let count = db_contract_count_by_token(&db, token) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; +pub fn contract_count_token(token: &str, conn: &State) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; + let count = db_contract_count_by_token(&db, token).map_err(db_error)?; Ok(Json(json!(count))) } @@ -112,21 +108,15 @@ pub fn contract_count_token(token: &str, conn: &State) -> Result /// Status: Deprecated /// (Needs to be split into interval rather than producing 3 fixed ones) #[get("/contract/volume?")] -pub fn contract_volume( - end: Option, - conn: &State, -) -> Result>, Custom> { +pub fn contract_volume(end: Option, conn: &State) -> ApiResult> { let end_timestamp = match end { Some(e) => e, None => time_now(), }; - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let db = conn.cauldron_r.get().map_err(db_error)?; let volume = super::contract_volume(&db, end_timestamp as u64) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + .map_err(|e| bad_request(ApiErrorCode::InvalidParameters, &e.to_string()))?; let result: Vec = volume .into_par_iter() diff --git a/src/rpc/err.rs b/src/rpc/err.rs index 367e1a4..2e16462 100644 --- a/src/rpc/err.rs +++ b/src/rpc/err.rs @@ -3,8 +3,113 @@ // This software is licensed under the GNU Affero General Public License (AGPL), version 3.0 or later. // A copy of the license can be found in the LICENSE file or at https://www.gnu.org/licenses/agpl-3.0.html -use rocket::{http::Status, response::status::Custom}; +use rocket::{http::Status, response::status::Custom, serde::json::Json}; +use serde_json::{json, Value}; +use std::fmt; +/// Standard API result type with JSON-formatted errors +pub type ApiResult = Result, Custom>>; + +/// API error codes for consistent error responses +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ApiErrorCode { + // 400 Bad Request codes + InvalidTimestamp, + InvalidTokenId, + InvalidPoolId, + InvalidTxid, + InvalidParameters, + InvalidTimeRange, + InvalidStepsize, + MissingParameters, + MissingCategory, + TooManyIntervals, + FutureTimestamp, + + // 404 Not Found codes + PriceNotFound, + PoolNotFound, + + // 500 Internal Server Error codes + InternalError, + + // 503 Service Unavailable codes + CacheWarming, +} + +impl fmt::Display for ApiErrorCode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let s = match self { + // 400 codes + Self::InvalidTimestamp => "INVALID_TIMESTAMP", + Self::InvalidTokenId => "INVALID_TOKEN_ID", + Self::InvalidPoolId => "INVALID_POOL_ID", + Self::InvalidTxid => "INVALID_TXID", + Self::InvalidParameters => "INVALID_PARAMETERS", + Self::InvalidTimeRange => "INVALID_TIME_RANGE", + Self::InvalidStepsize => "INVALID_STEPSIZE", + Self::MissingParameters => "MISSING_PARAMETERS", + Self::MissingCategory => "MISSING_CATEGORY", + Self::TooManyIntervals => "TOO_MANY_INTERVALS", + Self::FutureTimestamp => "FUTURE_TIMESTAMP", + // 404 codes + Self::PriceNotFound => "PRICE_NOT_FOUND", + Self::PoolNotFound => "POOL_NOT_FOUND", + // 500 codes + Self::InternalError => "INTERNAL_ERROR", + // 503 codes + Self::CacheWarming => "CACHE_WARMING", + }; + write!(f, "{s}") + } +} + +/// Create a 400 Bad Request error with JSON body +pub fn bad_request(code: ApiErrorCode, message: &str) -> Custom> { + Custom( + Status::BadRequest, + Json(json!({ + "error": { "code": code.to_string(), "message": message } + })), + ) +} + +/// Create a 404 Not Found error with JSON body +pub fn not_found(code: ApiErrorCode, message: &str) -> Custom> { + Custom( + Status::NotFound, + Json(json!({ + "error": { "code": code.to_string(), "message": message } + })), + ) +} + +/// Create a 500 Internal Server Error with JSON body +pub fn internal_error(message: &str) -> Custom> { + Custom( + Status::InternalServerError, + Json(json!({ + "error": { "code": ApiErrorCode::InternalError.to_string(), "message": message } + })), + ) +} + +/// Create a 500 Internal Server Error for database errors with JSON body +pub fn db_error(e: E) -> Custom> { + internal_error(&format!("Database error: {e}")) +} + +/// Create a 503 Service Unavailable error with JSON body +pub fn service_unavailable(code: ApiErrorCode, message: &str) -> Custom> { + Custom( + Status::ServiceUnavailable, + Json(json!({ + "error": { "code": code.to_string(), "message": message } + })), + ) +} + +// Legacy helper for backward compatibility during migration pub fn to_internal_error(e: E) -> Custom { Custom(Status::InternalServerError, format!("Error: {e}")) } diff --git a/src/rpc/oracle.rs b/src/rpc/oracle.rs index dc97ed5..2456aac 100644 --- a/src/rpc/oracle.rs +++ b/src/rpc/oracle.rs @@ -5,11 +5,10 @@ use crate::db::oracle::{get_closest, get_range, get_range_with_step}; use crate::db::DB; +use crate::rpc::err::{bad_request, db_error, ApiErrorCode, ApiResult}; use crate::timeutil::time_now; use bitcoin_hashes::hex::FromHex; use bitcoincash::TokenID; -use rocket::http::Status; -use rocket::response::status::Custom; use rocket::serde::json::Json; use rocket::{get, State}; use serde_json::{json, Value}; @@ -20,25 +19,28 @@ use serde_json::{json, Value}; /// /// - token_id: The 32 byte token ID /// - timestamp: Unix timestamp +/// +/// Returns `null` if no oracle data is found. #[get("/delphi/closest?&")] pub fn oracle_get_closest( token_id: Option, timestamp: Option, db: &State, -) -> Result, Custom> { +) -> ApiResult { let current_timestamp = timestamp.unwrap_or_else(time_now); - let conn = db - .oracle_r - .get() - .map_err(|e| Custom(Status::InternalServerError, e.to_string()))?; + let conn = db.oracle_r.get().map_err(db_error)?; let token_id = token_id .map(|t| { - TokenID::from_hex(&t) - .map_err(|e| Custom(Status::BadRequest, format!("Invalid token ID: {e}"))) + TokenID::from_hex(&t).map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token ID: {e}"), + ) + }) }) .transpose()?; - let entry = get_closest(&conn, &token_id, current_timestamp) - .map_err(|e| Custom(Status::InternalServerError, e.to_string()))?; + let entry = get_closest(&conn, &token_id, current_timestamp).map_err(db_error)?; + // Return null with 200 OK if no data found - this is not an error condition Ok(Json(entry.map_or(serde_json::Value::Null, |e| { serde_json::to_value(e).unwrap() }))) @@ -51,21 +53,21 @@ pub fn oracle_get_range( start: Option, end: Option, db: &State, -) -> Result>, Custom> { +) -> ApiResult> { let end_timestamp = end.unwrap_or_else(time_now); let start_timestamp = start.unwrap_or_else(|| end_timestamp - 86400); // 1 day in seconds - let conn = db - .oracle_r - .get() - .map_err(|e| Custom(Status::InternalServerError, e.to_string()))?; + let conn = db.oracle_r.get().map_err(db_error)?; let token_id = token_id .map(|t| { - TokenID::from_hex(&t) - .map_err(|e| Custom(Status::BadRequest, format!("Invalid token ID: {e}"))) + TokenID::from_hex(&t).map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token ID: {e}"), + ) + }) }) .transpose()?; - let entries = get_range(&conn, &token_id, start_timestamp, end_timestamp) - .map_err(|e| Custom(Status::InternalServerError, e.to_string()))?; + let entries = get_range(&conn, &token_id, start_timestamp, end_timestamp).map_err(db_error)?; Ok(Json( entries .into_iter() @@ -85,27 +87,35 @@ pub fn oracle_get_history( end: Option, stepsize: Option, db: &State, -) -> Result, Custom> { +) -> ApiResult { let current_timestamp = time_now(); - let conn = db - .oracle_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("DB error: {e}")))?; + let conn = db.oracle_r.get().map_err(db_error)?; - let token_id = token - .parse::() - .map_err(|e| Custom(Status::BadRequest, format!("Invalid token ID: {e}")))?; + let token_id = token.parse::().map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token ID: {e}"), + ) + })?; let start_ts = start.unwrap_or(current_timestamp - 30 * 24 * 3600); // default: 30 days ago let end_ts = end.unwrap_or(current_timestamp); let entries = if let Some(step) = stepsize { - get_range_with_step(&conn, &Some(token_id), start_ts, end_ts, step) - .map_err(|e| Custom(Status::BadRequest, format!("Query error: {e}")))? + get_range_with_step(&conn, &Some(token_id), start_ts, end_ts, step).map_err(|e| { + bad_request( + ApiErrorCode::InvalidParameters, + &format!("Query error: {e}"), + ) + })? } else { - get_range(&conn, &Some(token_id), start_ts, end_ts) - .map_err(|e| Custom(Status::BadRequest, format!("Query error: {e}")))? + get_range(&conn, &Some(token_id), start_ts, end_ts).map_err(|e| { + bad_request( + ApiErrorCode::InvalidParameters, + &format!("Query error: {e}"), + ) + })? }; let json_entries: Vec = entries diff --git a/src/rpc/pool.rs b/src/rpc/pool.rs index e7b9f5d..2ada417 100644 --- a/src/rpc/pool.rs +++ b/src/rpc/pool.rs @@ -15,13 +15,14 @@ use crate::{ DB, }, def::PoolID, + rpc::err::{bad_request, db_error, not_found, ApiErrorCode, ApiResult}, timeutil::time_now, }; use anyhow::Result; use bitcoin_hashes::hex::{FromHex, ToHex}; use bitcoincash::Txid; use riftenlabs_defi::chainutil::compute_outpoint_hash; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use rusqlite::{params, Connection}; use serde_json::{json, Value}; @@ -151,14 +152,10 @@ fn pools_by_apy(connection: &Connection) -> Result> { /// Status: Deprecated #[get("/pool/list_by_apy")] -pub fn list_pools_by_apy(conn: &State) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; +pub fn list_pools_by_apy(conn: &State) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; - let pools: Vec = - pools_by_apy(&db).map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + let pools: Vec = pools_by_apy(&db).map_err(db_error)?; Ok(Json(json!({ "pools": json!(pools) @@ -242,14 +239,14 @@ pub fn list_active_pools( token: Option<&str>, pkh: Option<&str>, conn: &State, -) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; +) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; if token.is_none() && pkh.is_none() { - return Err(Custom(Status::BadRequest, "Provide token or pkh".into())); + return Err(bad_request( + ApiErrorCode::MissingParameters, + "Provide token or pkh", + )); } let mut active_pool_list = ActivePoolList::default(); @@ -265,7 +262,7 @@ pub fn list_active_pools( timestamp_gte: None, }, ) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + .map_err(db_error)?; Ok(Json(json!({ "active": active_pool_list.active, @@ -273,26 +270,28 @@ pub fn list_active_pools( } #[get("/pool/history/?")] -pub fn pool_history( - pool_id: &str, - start: Option, - conn: &State, -) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; +pub fn pool_history(pool_id: &str, start: Option, conn: &State) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; let start = start.unwrap_or(time_now() as u64 - (30 * 3600 * 24) /* 30 days ago */); - let pool_id = PoolID::from_hex(pool_id) - .map_err(|e| Custom(Status::BadRequest, format!("Invalid pool ID: {e}")))?; + let pool_id = PoolID::from_hex(pool_id).map_err(|e| { + bad_request( + ApiErrorCode::InvalidPoolId, + &format!("Invalid pool ID: {e}"), + ) + })?; - let (token_id, owner_pkh) = db_pool_get_details(&db, &pool_id) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + let (token_id, owner_pkh) = db_pool_get_details(&db, &pool_id).map_err(|e| { + let err_msg = e.to_string(); + if err_msg.contains("not found") || err_msg.contains("no rows") { + not_found(ApiErrorCode::PoolNotFound, &format!("Pool not found: {e}")) + } else { + db_error(e) + } + })?; - let history = db_pool_history(&db, &pool_id, start) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let history = db_pool_history(&db, &pool_id, start).map_err(db_error)?; Ok(Json(json!({ "history": history, @@ -313,34 +312,26 @@ pub fn pool_history( /// } /// ``` #[get("/pool/id_from_utxo?&")] -pub fn pool_id_from_utxo( - txid: &str, - n: u32, - conn: &State, -) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; +pub fn pool_id_from_utxo(txid: &str, n: u32, conn: &State) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; // Parse txid from hex let txid = Txid::from_hex(txid) - .map_err(|e| Custom(Status::BadRequest, format!("Invalid txid: {e}")))?; + .map_err(|e| bad_request(ApiErrorCode::InvalidTxid, &format!("Invalid txid: {e}")))?; // Compute outpoint hash from txid and input position let utxo_hash = compute_outpoint_hash(&txid, n); // Query pool_history_entry table - let pool_id = db_pool_id_from_utxo(&db, &utxo_hash) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let pool_id = db_pool_id_from_utxo(&db, &utxo_hash).map_err(db_error)?; match pool_id { Some(pool_id) => Ok(Json(json!({ "pool_id": pool_id }))), - None => Err(Custom( - Status::NotFound, - format!( + None => Err(not_found( + ApiErrorCode::PoolNotFound, + &format!( "No pool found for UTXO: txid={}, input_pos={}", txid.to_hex(), n diff --git a/src/rpc/price.rs b/src/rpc/price.rs index 860c0eb..2cb863d 100644 --- a/src/rpc/price.rs +++ b/src/rpc/price.rs @@ -6,7 +6,7 @@ use anyhow::{bail, Context, Result}; use bitcoin_hashes::hex::{FromHex, ToHex}; use bitcoincash::TokenID; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use rusqlite::{params, Connection}; use rust_decimal::prelude::*; use serde_json::{json, Value}; @@ -18,7 +18,10 @@ use crate::{ }, DB, }, - rpc::tvl::get_token_tvl, + rpc::{ + err::{bad_request, db_error, not_found, ApiErrorCode, ApiResult}, + tvl::get_token_tvl, + }, timeutil::time_now, }; @@ -87,10 +90,27 @@ impl PriceInterval { } } +/// Error returned when no price data is available for a token +#[derive(Debug)] +pub struct NoPriceData; + +impl std::fmt::Display for NoPriceData { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "No price data available") + } +} + +impl std::error::Error for NoPriceData {} + // Get the current price of a given token fn current_price(db: &Connection, token_id: &str) -> Result { let (sats, tokens) = get_token_tvl(db, None, token_id)?; + // No TVL means no price data - return specific error + if tokens == 0 { + return Err(NoPriceData.into()); + } + let sum_sats = Decimal::from_u64(sats).context("overflow")?; let sum_tokens = Decimal::from_u64(tokens).context("overflow")?; @@ -268,42 +288,43 @@ pub fn price_at_or_before_2( /// } /// ``` #[get("/price//at/")] -pub fn price_at( - token: &str, - timestamp: &str, - conn: &State, -) -> Result, Custom> { +pub fn price_at(token: &str, timestamp: &str, conn: &State) -> ApiResult { let timestamp: i64 = timestamp.parse().map_err(|_| { - Custom( - Status::BadRequest, - "Invalid timestamp format. Must be a valid number.".to_string(), + bad_request( + ApiErrorCode::InvalidTimestamp, + "Invalid timestamp format. Must be a valid number.", ) })?; - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let db = conn.cauldron_r.get().map_err(db_error)?; let current_time = time_now(); if timestamp > current_time { - return Err(Custom( - Status::BadRequest, - "Timestamp is in the future".to_string(), + return Err(bad_request( + ApiErrorCode::FutureTimestamp, + "Timestamp is in the future", )); } - let token = - TokenID::from_hex(token).map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + let token = TokenID::from_hex(token).map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token ID: {e}"), + ) + })?; match price_at_or_before_2(&db, timestamp, &token) { Ok((latest_timestamp, price)) => Ok(Json(json!({ "timestamp": latest_timestamp, "price": price }))), - Err(e) => Err(Custom( - Status::InternalServerError, - format!("Error fetching price: {e}"), - )), + Err(e) => { + let err_msg = e.to_string(); + if err_msg.contains("No price data found") { + Err(not_found(ApiErrorCode::PriceNotFound, &err_msg)) + } else { + Err(db_error(e)) + } + } } } @@ -320,14 +341,19 @@ pub fn price_at( /// ``` /// #[get("/price//current")] -pub fn price_current(token: &str, conn: &State) -> Result, Custom> { - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; +pub fn price_current(token: &str, conn: &State) -> ApiResult { + let db = conn.cauldron_r.get().map_err(db_error)?; - let price = current_price(&db, token) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let price = current_price(&db, token).map_err(|e| { + if e.downcast_ref::().is_some() { + not_found( + ApiErrorCode::PriceNotFound, + "No price data available for this token", + ) + } else { + db_error(e) + } + })?; Ok(Json(json!({ "price": price, @@ -376,13 +402,10 @@ pub fn price_history( end: Option, stepsize: Option, conn: &State, -) -> Result, Custom> { +) -> ApiResult { let current_timestamp = time_now(); - let db = conn - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + let db = conn.cauldron_r.get().map_err(db_error)?; let history = historic_price( &db, @@ -391,7 +414,7 @@ pub fn price_history( stepsize.unwrap_or(3600 /* 1 hour */), token, ) - .map_err(|e| Custom(Status::BadRequest, format!("Error: {e}")))?; + .map_err(|e| bad_request(ApiErrorCode::InvalidParameters, &e.to_string()))?; let history_json: Vec = history .iter() @@ -768,6 +791,11 @@ mod tests { .get(format!("/cauldron/price/{bad_token_id}/at/{timestamp}")) .dispatch(); assert_eq!(response.status(), Status::BadRequest); + + // Verify JSON error format + let body = response.into_string().unwrap(); + let json: serde_json::Value = serde_json::from_str(&body).expect("Response should be JSON"); + assert!(json["error"]["code"].as_str().is_some()); } #[test] @@ -789,7 +817,12 @@ mod tests { assert_eq!(response.status(), Status::BadRequest); let body = response.into_string().unwrap(); - assert!(body.contains("Timestamp is in the future")); + let json: serde_json::Value = serde_json::from_str(&body).expect("Response should be JSON"); + assert_eq!(json["error"]["code"], "FUTURE_TIMESTAMP"); + assert!(json["error"]["message"] + .as_str() + .unwrap() + .contains("future")); } #[test] @@ -812,7 +845,12 @@ mod tests { assert_eq!(response.status(), Status::BadRequest); let body = response.into_string().unwrap(); - assert!(body.contains("Invalid timestamp format")); + let json: serde_json::Value = serde_json::from_str(&body).expect("Response should be JSON"); + assert_eq!(json["error"]["code"], "INVALID_TIMESTAMP"); + assert!(json["error"]["message"] + .as_str() + .unwrap() + .contains("Invalid timestamp format")); } #[test] fn test_price_with_high_tokens_and_low_sats() { @@ -851,6 +889,10 @@ mod tests { .dispatch(); // Expect that we could not handle the calculation/conversion. - assert_eq!(response.status(), Status::InternalServerError); + // This returns 404 NOT_FOUND since no valid price can be computed + assert_eq!(response.status(), Status::NotFound); + let body = response.into_string().unwrap(); + let json: serde_json::Value = serde_json::from_str(&body).expect("Response should be JSON"); + assert!(json["error"]["code"].as_str().is_some()); } } diff --git a/src/rpc/tokens.rs b/src/rpc/tokens.rs index 2e0922b..e1f25b3 100644 --- a/src/rpc/tokens.rs +++ b/src/rpc/tokens.rs @@ -6,8 +6,9 @@ use bitcoin_hashes::hex::{FromHex, ToHex}; use bitcoincash::TokenID; use log::{info, warn}; -use rocket::http::Status; -use rocket::{get, response::status::Custom, serde::json::Json, State}; +use rocket::response::status::Custom; +use rocket::serde::json::Json; +use rocket::{get, State}; use serde_json::json; use serde_json::Value; @@ -25,7 +26,10 @@ use crate::rpc::ResponseCacheInner; use crate::timeutil::time_now; use rayon::prelude::*; -use super::err::to_internal_error; +use super::err::{ + bad_request, db_error, not_found, service_unavailable, to_internal_error, ApiErrorCode, + ApiResult, +}; use super::ResponseCache; macro_rules! function_name { @@ -40,10 +44,7 @@ macro_rules! function_name { } #[get("/tokens/list_by_volume")] -pub fn list_by_volume( - db: &State, - response_cache: &State, -) -> Result, Custom> { +pub fn list_by_volume(db: &State, response_cache: &State) -> ApiResult { let thirty_days = 24 * 60 * 60 * 30; let duration = thirty_days; let limit = 250; @@ -115,23 +116,20 @@ pub fn list_by_volume( match cached_value { Some(r) => Ok(Json(r)), - None => Err(Custom( - Status::ServiceUnavailable, - "Busy, try again in 30 seconds".to_string(), + None => Err(service_unavailable( + ApiErrorCode::CacheWarming, + "Busy, try again in 30 seconds", )), } } #[get("/tokens/search_by_volume?")] -pub fn search_by_volume( - search_query: &str, - db: &State, -) -> Result>, Custom> { +pub fn search_by_volume(search_query: &str, db: &State) -> ApiResult> { let db_copy = db.inner().clone(); let run_query = move || { #[allow(clippy::type_complexity)] - let bcmr_db = db_copy.bcmr_r.get().map_err(to_internal_error)?; - let crc20_db = db_copy.crc20_r.get().map_err(to_internal_error)?; + let bcmr_db = db_copy.bcmr_r.get().map_err(db_error)?; + let crc20_db = db_copy.crc20_r.get().map_err(db_error)?; let list: Vec<(String, Option, Option, u64)> = db::search::search_tokens_by_volume( @@ -140,7 +138,7 @@ pub fn search_by_volume( &crc20_db, search_query, ) - .map_err(|e| Custom(Status::InternalServerError, format!("Error: {e}")))?; + .map_err(db_error)?; let result: Vec = list .into_par_iter() @@ -166,9 +164,9 @@ pub fn search_cached( offset: Option, by: Option, order: Option, -) -> Result, Custom> { - let cauldron_db = db.cauldron_r.get().map_err(to_internal_error)?; - let bcmr_db = db.bcmr_r.get().map_err(to_internal_error)?; +) -> ApiResult { + let cauldron_db = db.cauldron_r.get().map_err(db_error)?; + let bcmr_db = db.bcmr_r.get().map_err(db_error)?; let sort = parse_cached_sort(by, order); let limit = limit.unwrap_or(250); @@ -177,7 +175,7 @@ pub fn search_cached( let items: Vec = db_search_tokens_cached(&cauldron_db, &bcmr_db, &query, sort, limit, offset) - .map_err(to_internal_error)?; + .map_err(db_error)?; Ok(Json(json!(items))) } @@ -278,17 +276,16 @@ pub fn list_cached( offset: Option, by: Option, order: Option, -) -> Result, Custom> { +) -> ApiResult { let limit = limit.unwrap_or(250); let offset = offset.unwrap_or(0); - let cauldron_db = db.cauldron_r.get().map_err(to_internal_error)?; - let bcmr_db = db.bcmr_r.get().map_err(to_internal_error)?; + let cauldron_db = db.cauldron_r.get().map_err(db_error)?; + let bcmr_db = db.bcmr_r.get().map_err(db_error)?; let sort = parse_cached_sort(by, order); let items: Vec = - db_list_tokens_cached(&cauldron_db, &bcmr_db, limit, offset, sort) - .map_err(to_internal_error)?; + db_list_tokens_cached(&cauldron_db, &bcmr_db, limit, offset, sort).map_err(db_error)?; Ok(Json(json!(items))) } @@ -299,9 +296,9 @@ pub fn list_cached_by_ids( ids: &str, by: Option, order: Option, -) -> Result, Custom> { - let cauldron_db = db.cauldron_r.get().map_err(to_internal_error)?; - let bcmr_db = db.bcmr_r.get().map_err(to_internal_error)?; +) -> ApiResult { + let cauldron_db = db.cauldron_r.get().map_err(db_error)?; + let bcmr_db = db.bcmr_r.get().map_err(db_error)?; // split, trim, and normalize (lowercase is typical for hex IDs in DB) let token_ids: Vec = ids @@ -315,30 +312,28 @@ pub fn list_cached_by_ids( } let sort = parse_cached_sort(by, order); - let items = db_list_tokens_cached_by_ids(&cauldron_db, &bcmr_db, &token_ids, sort) - .map_err(to_internal_error)?; + let items = + db_list_tokens_cached_by_ids(&cauldron_db, &bcmr_db, &token_ids, sort).map_err(db_error)?; Ok(Json(json!(items))) } #[get("/token//first_pool")] -pub fn first_pool_creation(token: &str, dbp: &State) -> Result, Custom> { - let token = TokenID::from_hex(token) - .map_err(|e| Custom(Status::BadRequest, format!("Invalid token id: {e}")))?; +pub fn first_pool_creation(token: &str, dbp: &State) -> ApiResult { + let token = TokenID::from_hex(token).map_err(|e| { + bad_request( + ApiErrorCode::InvalidTokenId, + &format!("Invalid token id: {e}"), + ) + })?; let token_hex = token.to_hex(); - let conn = dbp - .cauldron_r - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("DB error: {e}")))?; + let conn = dbp.cauldron_r.get().map_err(db_error)?; match db_first_pool_creation_row(&conn, &token_hex) { Ok(Some((creation_utxo, txid, timestamp, block_height))) => { // Opportunistically cache the timestamp forever - let cw = dbp - .cauldron_w - .get() - .map_err(|e| Custom(Status::InternalServerError, format!("DB error: {e}")))?; + let cw = dbp.cauldron_w.get().map_err(db_error)?; if let Err(e) = cache_first_pool_ts_if_empty(&cw, &token_hex, timestamp) { // Non-fatal: return the data log::warn!("Failed to cache first_pool_ts for {token_hex}: {e}"); @@ -352,7 +347,7 @@ pub fn first_pool_creation(token: &str, dbp: &State) -> Result, "block_height": block_height }))) } - Ok(None) => Err(Custom(Status::NotFound, "No pools for token".into())), - Err(e) => Err(Custom(Status::InternalServerError, format!("Error: {e}"))), + Ok(None) => Err(not_found(ApiErrorCode::PoolNotFound, "No pools for token")), + Err(e) => Err(db_error(e)), } } diff --git a/src/rpc/tvl.rs b/src/rpc/tvl.rs index e214ff6..50b352a 100644 --- a/src/rpc/tvl.rs +++ b/src/rpc/tvl.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use anyhow::Result; use rayon::iter::{IntoParallelIterator, ParallelIterator}; -use rocket::{get, http::Status, response::status::Custom, serde::json::Json, State}; +use rocket::{get, serde::json::Json, State}; use rusqlite::Connection; use serde_json::{json, Value}; @@ -17,6 +17,7 @@ use crate::db::{ }, DB, }; +use crate::rpc::err::{db_error, ApiResult}; #[derive(Default)] pub struct TvlByTokenVisitor { @@ -149,14 +150,11 @@ pub fn get_token_tvl( /// use valuelocked with optional parameters /// used by defilama; fix adapter before removing #[get("/tvl/