From 3ba48807683766836d7edae3cdccb2bf446109c0 Mon Sep 17 00:00:00 2001 From: Damon Sicore Date: Tue, 19 Aug 2025 15:29:25 -0500 Subject: [PATCH 1/3] Add versioned REST API with improved error codes --- node/rest/src/helpers/error.rs | 40 ++++++++++-- node/rest/src/lib.rs | 57 ++++++++++------- node/rest/src/routes.rs | 111 +++++++++++++++++++++++---------- 3 files changed, 151 insertions(+), 57 deletions(-) diff --git a/node/rest/src/helpers/error.rs b/node/rest/src/helpers/error.rs index 8b76b1f0b0..5d034533d5 100644 --- a/node/rest/src/helpers/error.rs +++ b/node/rest/src/helpers/error.rs @@ -18,17 +18,49 @@ use axum::{ response::{IntoResponse, Response}, }; -/// An enum of error handlers for the REST API server. -pub struct RestError(pub String); +/// A generic error for the REST API server. +pub struct RestError { + /// The HTTP status code to return. + pub status: StatusCode, + /// The error message. + pub message: String, +} + +impl RestError { + /// Creates a new internal server error. + pub fn new(message: impl Into) -> Self { + Self { status: StatusCode::INTERNAL_SERVER_ERROR, message: message.into() } + } + + /// Creates a new error with a specific status code. + pub fn with_status(message: impl Into, status: StatusCode) -> Self { + Self { status, message: message.into() } + } + + /// Creates a 400 Bad Request error. + pub fn bad_request(message: impl Into) -> Self { + Self::with_status(message, StatusCode::BAD_REQUEST) + } + + /// Creates a 404 Not Found error. + pub fn not_found(message: impl Into) -> Self { + Self::with_status(message, StatusCode::NOT_FOUND) + } + + /// Creates a 503 Service Unavailable error. + pub fn service_unavailable(message: impl Into) -> Self { + Self::with_status(message, StatusCode::SERVICE_UNAVAILABLE) + } +} impl IntoResponse for RestError { fn into_response(self) -> Response { - (StatusCode::INTERNAL_SERVER_ERROR, format!("Something went wrong: {}", self.0)).into_response() + (self.status, format!("Something went wrong: {}", self.message)).into_response() } } impl From for RestError { fn from(err: anyhow::Error) -> Self { - Self(err.to_string()) + Self::new(err.to_string()) } } diff --git a/node/rest/src/lib.rs b/node/rest/src/lib.rs index e1f68ba403..fcd9285ed7 100644 --- a/node/rest/src/lib.rs +++ b/node/rest/src/lib.rs @@ -140,25 +140,6 @@ impl, R: Routing> Rest { // Log the REST rate limit per IP. debug!("REST rate limit per IP - {rest_rps} RPS"); - // Prepare the rate limiting setup. - let governor_config = Box::new( - GovernorConfigBuilder::default() - .per_nanosecond((1_000_000_000 / rest_rps) as u64) - .burst_size(rest_rps) - .error_handler(|error| { - // Properly return a 429 Too Many Requests error - let error_message = error.to_string(); - let mut response = Response::new(error_message.clone().into()); - *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - if error_message.contains("Too Many Requests") { - *response.status_mut() = StatusCode::TOO_MANY_REQUESTS; - } - response - }) - .finish() - .expect("Couldn't set up rate limiting for the REST server!"), - ); - // Get the network being used. let network = match N::ID { snarkvm::console::network::MainnetV0::ID => "mainnet", @@ -169,7 +150,27 @@ impl, R: Routing> Rest { } }; - let router = { + // Closure to build the API routes for a given version. + let build_routes = || { + // Prepare the rate limiting setup. + let governor_config = Box::new( + GovernorConfigBuilder::default() + .per_nanosecond((1_000_000_000 / rest_rps) as u64) + .burst_size(rest_rps) + .error_handler(|error| { + // Properly return a 429 Too Many Requests error + let error_message = error.to_string(); + let mut response = Response::new(error_message.clone().into()); + *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; + if error_message.contains("Too Many Requests") { + *response.status_mut() = StatusCode::TOO_MANY_REQUESTS; + } + response + }) + .finish() + .expect("Couldn't set up rate limiting for the REST server!"), + ); + let routes = axum::Router::new() // All the endpoints before the call to `route_layer` are protected with JWT auth. @@ -266,7 +267,7 @@ impl, R: Routing> Rest { // Custom logging. .layer(middleware::from_fn(log_middleware)) // Enable CORS. - .layer(cors) + .layer(cors.clone()) // Cap the request body size at 512KiB. .layer(DefaultBodyLimit::max(512 * 1024)) .layer(GovernorLayer { @@ -274,6 +275,11 @@ impl, R: Routing> Rest { }) }; + // Build routers for v1 and v2. + let router_v1 = build_routes().route_layer(middleware::from_fn(legacy_error_middleware)); + let router_v2 = axum::Router::new().nest("/v2", build_routes()); + let router = router_v1.merge(router_v2); + let rest_listener = TcpListener::bind(rest_ip).await.with_context(|| "Failed to bind TCP port for REST endpoints")?; @@ -307,3 +313,12 @@ pub fn fmt_id(id: impl ToString) -> String { } formatted_id } + +/// Middleware to ensure legacy routes always return HTTP 500 on error. +async fn legacy_error_middleware(req: Request, next: Next) -> Response { + let mut res = next.run(req).await; + if !res.status().is_success() { + *res.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; + } + res +} diff --git a/node/rest/src/routes.rs b/node/rest/src/routes.rs index 0be8597ae8..91ba120b2c 100644 --- a/node/rest/src/routes.rs +++ b/node/rest/src/routes.rs @@ -112,13 +112,19 @@ impl, R: Routing> Rest { // Manually parse the height or the height of the hash, axum doesn't support different types // for the same path param. let block = if let Ok(height) = height_or_hash.parse::() { - rest.ledger.get_block(height)? + rest + .ledger + .get_block(height) + .map_err(|_| RestError::not_found(format!("Block {height} not found")))? } else { let hash = height_or_hash .parse::() - .map_err(|_| RestError("invalid input, it is neither a block height nor a block hash".to_string()))?; + .map_err(|_| RestError::bad_request("invalid input, it is neither a block height nor a block hash"))?; - rest.ledger.get_block_by_hash(&hash)? + rest + .ledger + .get_block_by_hash(&hash) + .map_err(|_| RestError::not_found(format!("Block {hash} not found")))? }; Ok(ErasedJson::pretty(block)) @@ -136,12 +142,12 @@ impl, R: Routing> Rest { // Ensure the end height is greater than the start height. if start_height > end_height { - return Err(RestError("Invalid block range".to_string())); + return Err(RestError::bad_request("Invalid block range")); } // Ensure the block range is bounded. if end_height - start_height > MAX_BLOCK_RANGE { - return Err(RestError(format!( + return Err(RestError::bad_request(format!( "Cannot request more than {MAX_BLOCK_RANGE} blocks per call (requested {})", end_height - start_height ))); @@ -150,7 +156,7 @@ impl, R: Routing> Rest { // Prepare a closure for the blocking work. let get_json_blocks = move || -> Result { let blocks = cfg_into_iter!(start_height..end_height) - .map(|height| rest.ledger.get_block(height)) + .map(|height| rest.ledger.get_block(height).map_err(|_| RestError::not_found(format!("Block {height} not found")))) .collect::, _>>()?; Ok(ErasedJson::pretty(blocks)) @@ -159,7 +165,9 @@ impl, R: Routing> Rest { // Fetch the blocks from ledger and serialize to json. match tokio::task::spawn_blocking(get_json_blocks).await { Ok(json) => json, - Err(err) => Err(RestError(format!("Failed to get blocks '{start_height}..{end_height}' - {err}"))), + Err(err) => Err(RestError::not_found(format!( + "Failed to get blocks '{start_height}..{end_height}' - {err}" + ))), } } @@ -215,7 +223,9 @@ impl, R: Routing> Rest { State(rest): State, Path(hash): Path, ) -> Result { - Ok(ErasedJson::pretty(rest.ledger.get_height(&hash)?)) + Ok(ErasedJson::pretty( + rest.ledger.get_height(&hash).map_err(|_| RestError::not_found(format!("Block {hash} not found")))?, + )) } // GET //block/{height}/header @@ -223,7 +233,9 @@ impl, R: Routing> Rest { State(rest): State, Path(height): Path, ) -> Result { - Ok(ErasedJson::pretty(rest.ledger.get_header(height)?)) + Ok(ErasedJson::pretty( + rest.ledger.get_header(height).map_err(|_| RestError::not_found(format!("Block {height} not found")))?, + )) } // GET //block/{height}/transactions @@ -231,7 +243,11 @@ impl, R: Routing> Rest { State(rest): State, Path(height): Path, ) -> Result { - Ok(ErasedJson::pretty(rest.ledger.get_transactions(height)?)) + Ok(ErasedJson::pretty( + rest.ledger + .get_transactions(height) + .map_err(|_| RestError::not_found(format!("Block {height} not found")))?, + )) } // GET //transaction/{transactionID} @@ -239,7 +255,11 @@ impl, R: Routing> Rest { State(rest): State, Path(tx_id): Path, ) -> Result { - Ok(ErasedJson::pretty(rest.ledger.get_transaction(tx_id)?)) + Ok(ErasedJson::pretty( + rest.ledger + .get_transaction(tx_id) + .map_err(|_| RestError::not_found(format!("Transaction {tx_id} not found")))?, + )) } // GET //transaction/confirmed/{transactionID} @@ -247,7 +267,11 @@ impl, R: Routing> Rest { State(rest): State, Path(tx_id): Path, ) -> Result { - Ok(ErasedJson::pretty(rest.ledger.get_confirmed_transaction(tx_id)?)) + Ok(ErasedJson::pretty( + rest.ledger + .get_confirmed_transaction(tx_id) + .map_err(|_| RestError::not_found(format!("Transaction {tx_id} not found")))?, + )) } // GET //transaction/unconfirmed/{transactionID} @@ -255,7 +279,11 @@ impl, R: Routing> Rest { State(rest): State, Path(tx_id): Path, ) -> Result { - Ok(ErasedJson::pretty(rest.ledger.get_unconfirmed_transaction(&tx_id)?)) + Ok(ErasedJson::pretty( + rest.ledger + .get_unconfirmed_transaction(&tx_id) + .map_err(|_| RestError::not_found(format!("Transaction {tx_id} not found")))?, + )) } // GET //memoryPool/transmissions @@ -264,7 +292,7 @@ impl, R: Routing> Rest { Some(consensus) => { Ok(ErasedJson::pretty(consensus.unconfirmed_transmissions().collect::>())) } - None => Err(RestError("Route isn't available for this node type".to_string())), + None => Err(RestError::not_found("Route isn't available for this node type")), } } @@ -272,7 +300,7 @@ impl, R: Routing> Rest { pub(crate) async fn get_memory_pool_solutions(State(rest): State) -> Result { match rest.consensus { Some(consensus) => Ok(ErasedJson::pretty(consensus.unconfirmed_solutions().collect::>())), - None => Err(RestError("Route isn't available for this node type".to_string())), + None => Err(RestError::not_found("Route isn't available for this node type")), } } @@ -280,7 +308,7 @@ impl, R: Routing> Rest { pub(crate) async fn get_memory_pool_transactions(State(rest): State) -> Result { match rest.consensus { Some(consensus) => Ok(ErasedJson::pretty(consensus.unconfirmed_transactions().collect::>())), - None => Err(RestError("Route isn't available for this node type".to_string())), + None => Err(RestError::not_found("Route isn't available for this node type")), } } @@ -392,7 +420,9 @@ impl, R: Routing> Rest { ) -> Result { // Return an error if the `all` query parameter is not set to `true`. if metadata.all != Some(true) { - return Err(RestError("Invalid query parameter. At this time, 'all=true' must be included".to_string())); + return Err(RestError::bad_request( + "Invalid query parameter. At this time, 'all=true' must be included", + )); } // Retrieve the latest height. @@ -414,8 +444,8 @@ impl, R: Routing> Rest { // Return the full mapping without metadata. Ok(ErasedJson::pretty(mapping_values)) } - Ok(Err(err)) => Err(RestError(format!("Unable to read mapping - {err}"))), - Err(err) => Err(RestError(format!("Unable to read mapping - {err}"))), + Ok(Err(err)) => Err(RestError::not_found(format!("Unable to read mapping - {err}"))), + Err(err) => Err(RestError::not_found(format!("Unable to read mapping - {err}"))), } } @@ -460,14 +490,20 @@ impl, R: Routing> Rest { ) -> Result { // Do not process the request if the node is too far behind to avoid sending outdated data. if !rest.routing.is_within_sync_leniency() { - return Err(RestError("Unable to request delegators (node is syncing)".to_string())); + return Err(RestError::service_unavailable( + "Unable to request delegators (node is syncing)", + )); } // Return the delegators for the given validator. match tokio::task::spawn_blocking(move || rest.ledger.get_delegators_for_validator(&validator)).await { Ok(Ok(delegators)) => Ok(ErasedJson::pretty(delegators)), - Ok(Err(err)) => Err(RestError(format!("Unable to request delegators - {err}"))), - Err(err) => Err(RestError(format!("Unable to request delegators - {err}"))), + Ok(Err(err)) => Err(RestError::service_unavailable(format!( + "Unable to request delegators - {err}" + ))), + Err(err) => Err(RestError::service_unavailable(format!( + "Unable to request delegators - {err}" + ))), } } @@ -548,7 +584,10 @@ impl, R: Routing> Rest { ) -> Result { // Do not process the transaction if the node is too far behind. if !rest.routing.is_within_sync_leniency() { - return Err(RestError(format!("Unable to broadcast transaction '{}' (node is syncing)", fmt_id(tx.id())))); + return Err(RestError::service_unavailable(format!( + "Unable to broadcast transaction '{}' (node is syncing)", + fmt_id(tx.id()) + ))); } // If the transaction exceeds the transaction size limit, return an error. @@ -557,7 +596,7 @@ impl, R: Routing> Rest { // TODO: Should this be a blocking task? let buffer = Vec::with_capacity(3000); if tx.write_le(LimitedWriter::new(buffer, N::MAX_TRANSACTION_SIZE)).is_err() { - return Err(RestError("Transaction size exceeds the byte limit".to_string())); + return Err(RestError::bad_request("Transaction size exceeds the byte limit")); } if check_transaction.check_transaction.unwrap_or(false) { @@ -581,13 +620,13 @@ impl, R: Routing> Rest { let prev = counter.fetch_add(1, Ordering::Relaxed); if prev >= limit { counter.fetch_sub(1, Ordering::Relaxed); - return Err(RestError(err_msg.to_string())); + return Err(RestError::service_unavailable(err_msg.to_string())); } // Perform the check. let res = rest .ledger .check_transaction_basic(&tx, None, &mut rand::thread_rng()) - .map_err(|e| RestError(format!("Invalid transaction: {e}"))); + .map_err(|e| RestError::bad_request(format!("Invalid transaction: {e}"))); // Release the slot. counter.fetch_sub(1, Ordering::Relaxed); // Propagate error if any. @@ -620,7 +659,7 @@ impl, R: Routing> Rest { ) -> Result { // Do not process the solution if the node is too far behind. if !rest.routing.is_within_sync_leniency() { - return Err(RestError(format!( + return Err(RestError::service_unavailable(format!( "Unable to broadcast solution '{}' (node is syncing)", fmt_id(solution.id()) ))); @@ -644,7 +683,7 @@ impl, R: Routing> Rest { // here prevents the to-be aborted solutions from propagating through the network. let prover_address = solution.address(); if rest.ledger.is_solution_limit_reached(&prover_address, 0) { - return Err(RestError(format!( + return Err(RestError::bad_request(format!( "Invalid solution '{}' - Prover '{prover_address}' has reached their solution limit for the current epoch", fmt_id(solution.id()) ))); @@ -655,9 +694,17 @@ impl, R: Routing> Rest { { Ok(Ok(())) => {} Ok(Err(err)) => { - return Err(RestError(format!("Invalid solution '{}' - {err}", fmt_id(solution.id())))); + return Err(RestError::bad_request(format!( + "Invalid solution '{}' - {err}", + fmt_id(solution.id()) + ))); + } + Err(err) => { + return Err(RestError::bad_request(format!( + "Invalid solution '{}' - {err}", + fmt_id(solution.id()) + ))); } - Err(err) => return Err(RestError(format!("Invalid solution '{}' - {err}", fmt_id(solution.id())))), } } } @@ -693,7 +740,7 @@ impl, R: Routing> Rest { let history = snarkvm::synthesizer::History::new(N::ID, rest.ledger.vm().finalize_store().storage_mode()); let result = history .load_mapping(height, mapping) - .map_err(|_| RestError(format!("Could not load mapping '{mapping}' from block '{height}'")))?; + .map_err(|_| RestError::not_found(format!("Could not load mapping '{mapping}' from block '{height}'")))?; Ok((StatusCode::OK, [(CONTENT_TYPE, "application/json")], result)) } @@ -727,7 +774,7 @@ impl, R: Routing> Rest { Ok(ErasedJson::pretty(participation_scores)) } - None => Err(RestError("Route isn't available for this node type".to_string())), + None => Err(RestError::not_found("Route isn't available for this node type")), } } } From ce673e35175f96030ea9c1e005e7c97c4b444b66 Mon Sep 17 00:00:00 2001 From: Damon Sicore Date: Tue, 19 Aug 2025 16:44:08 -0500 Subject: [PATCH 2/3] Refine REST error handling and add versioned route tests --- Cargo.lock | 26 ++++++++--- node/rest/Cargo.toml | 4 ++ node/rest/src/helpers/error.rs | 14 +++--- node/rest/src/lib.rs | 81 +++++++++++++++++++++++++++++++--- node/rest/src/routes.rs | 52 +++++++++------------- 5 files changed, 127 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a60901cc11..f50963313f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -288,7 +288,7 @@ dependencies = [ "serde_urlencoded", "sync_wrapper", "tokio", - "tower", + "tower 0.5.2", "tower-layer", "tower-service", "tracing", @@ -333,7 +333,7 @@ dependencies = [ "rustversion", "serde", "serde_json", - "tower", + "tower 0.5.2", "tower-layer", "tower-service", "typed-json", @@ -3215,7 +3215,7 @@ dependencies = [ "tokio", "tokio-native-tls", "tokio-rustls", - "tower", + "tower 0.5.2", "tower-http", "tower-service", "url", @@ -4025,6 +4025,7 @@ dependencies = [ "snarkvm", "time", "tokio", + "tower 0.4.13", "tower-http", "tower_governor", "tracing", @@ -5590,6 +5591,21 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" +[[package]] +name = "tower" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8fa9be0de6cf49e536ce1851f987bd21a43b771b09473c3549a6c853db37c1c" +dependencies = [ + "futures-core", + "futures-util", + "pin-project", + "pin-project-lite", + "tower-layer", + "tower-service", + "tracing", +] + [[package]] name = "tower" version = "0.5.2" @@ -5628,7 +5644,7 @@ dependencies = [ "pin-project-lite", "tokio", "tokio-util", - "tower", + "tower 0.5.2", "tower-layer", "tower-service", "tracing", @@ -5658,7 +5674,7 @@ dependencies = [ "http 1.3.1", "pin-project", "thiserror 2.0.14", - "tower", + "tower 0.5.2", "tracing", ] diff --git a/node/rest/Cargo.toml b/node/rest/Cargo.toml index 1984263dc9..f5ac1621ca 100644 --- a/node/rest/Cargo.toml +++ b/node/rest/Cargo.toml @@ -114,5 +114,9 @@ workspace = true [dev-dependencies.base64] workspace = true +[dev-dependencies.tower] +version = "0.4" +features = ["util"] + [build-dependencies.built] workspace = true diff --git a/node/rest/src/helpers/error.rs b/node/rest/src/helpers/error.rs index 5d034533d5..bc15b3fdcd 100644 --- a/node/rest/src/helpers/error.rs +++ b/node/rest/src/helpers/error.rs @@ -28,27 +28,27 @@ pub struct RestError { impl RestError { /// Creates a new internal server error. - pub fn new(message: impl Into) -> Self { - Self { status: StatusCode::INTERNAL_SERVER_ERROR, message: message.into() } + pub fn new(message: String) -> Self { + Self { status: StatusCode::INTERNAL_SERVER_ERROR, message } } /// Creates a new error with a specific status code. - pub fn with_status(message: impl Into, status: StatusCode) -> Self { - Self { status, message: message.into() } + pub fn with_status(message: String, status: StatusCode) -> Self { + Self { status, message } } /// Creates a 400 Bad Request error. - pub fn bad_request(message: impl Into) -> Self { + pub fn bad_request(message: String) -> Self { Self::with_status(message, StatusCode::BAD_REQUEST) } /// Creates a 404 Not Found error. - pub fn not_found(message: impl Into) -> Self { + pub fn not_found(message: String) -> Self { Self::with_status(message, StatusCode::NOT_FOUND) } /// Creates a 503 Service Unavailable error. - pub fn service_unavailable(message: impl Into) -> Self { + pub fn service_unavailable(message: String) -> Self { Self::with_status(message, StatusCode::SERVICE_UNAVAILABLE) } } diff --git a/node/rest/src/lib.rs b/node/rest/src/lib.rs index fcd9285ed7..675d78fd02 100644 --- a/node/rest/src/lib.rs +++ b/node/rest/src/lib.rs @@ -132,11 +132,6 @@ impl, R: Routing> Rest { impl, R: Routing> Rest { async fn spawn_server(&mut self, rest_ip: SocketAddr, rest_rps: u32) -> Result<()> { - let cors = CorsLayer::new() - .allow_origin(Any) - .allow_methods([Method::GET, Method::POST, Method::OPTIONS]) - .allow_headers([CONTENT_TYPE]); - // Log the REST rate limit per IP. debug!("REST rate limit per IP - {rest_rps} RPS"); @@ -152,6 +147,11 @@ impl, R: Routing> Rest { // Closure to build the API routes for a given version. let build_routes = || { + let cors = CorsLayer::new() + .allow_origin(Any) + .allow_methods([Method::GET, Method::POST, Method::OPTIONS]) + .allow_headers([CONTENT_TYPE]); + // Prepare the rate limiting setup. let governor_config = Box::new( GovernorConfigBuilder::default() @@ -267,7 +267,7 @@ impl, R: Routing> Rest { // Custom logging. .layer(middleware::from_fn(log_middleware)) // Enable CORS. - .layer(cors.clone()) + .layer(cors) // Cap the request body size at 512KiB. .layer(DefaultBodyLimit::max(512 * 1024)) .layer(GovernorLayer { @@ -315,10 +315,77 @@ pub fn fmt_id(id: impl ToString) -> String { } /// Middleware to ensure legacy routes always return HTTP 500 on error. -async fn legacy_error_middleware(req: Request, next: Next) -> Response { +async fn legacy_error_middleware(req: Request, next: Next) -> Response { let mut res = next.run(req).await; if !res.status().is_success() { *res.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; } res } + +#[cfg(test)] +mod tests { + use super::*; + use axum::{ + Router, + body::Body, + http::{Request, StatusCode}, + middleware, + routing::get, + }; + use tower::ServiceExt; // for `oneshot` + + fn test_app() -> Router { + let build_routes = || { + Router::new() + .route( + "/not_found", + get(|| async { Err::<(), RestError>(RestError::not_found("missing".to_string())) }), + ) + .route( + "/bad_request", + get(|| async { Err::<(), RestError>(RestError::bad_request("bad".to_string())) }), + ) + .route( + "/service_unavailable", + get(|| async { Err::<(), RestError>(RestError::service_unavailable("gone".to_string())) }), + ) + }; + let router_v1 = build_routes().route_layer(middleware::from_fn(legacy_error_middleware)); + let router_v2 = Router::new().nest("/v2", build_routes()); + router_v1.merge(router_v2) + } + + #[tokio::test] + async fn v1_routes_force_internal_server_error() { + let app = test_app(); + + let res = app.clone().oneshot(Request::builder().uri("/not_found").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let res = + app.clone().oneshot(Request::builder().uri("/bad_request").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let res = + app.oneshot(Request::builder().uri("/service_unavailable").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); + } + + #[tokio::test] + async fn v2_routes_return_specific_errors() { + let app = test_app(); + + let res = + app.clone().oneshot(Request::builder().uri("/v2/not_found").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = + app.clone().oneshot(Request::builder().uri("/v2/bad_request").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); + + let res = + app.oneshot(Request::builder().uri("/v2/service_unavailable").body(Body::empty()).unwrap()).await.unwrap(); + assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE); + } +} diff --git a/node/rest/src/routes.rs b/node/rest/src/routes.rs index 91ba120b2c..d95cb18832 100644 --- a/node/rest/src/routes.rs +++ b/node/rest/src/routes.rs @@ -112,19 +112,13 @@ impl, R: Routing> Rest { // Manually parse the height or the height of the hash, axum doesn't support different types // for the same path param. let block = if let Ok(height) = height_or_hash.parse::() { - rest - .ledger - .get_block(height) - .map_err(|_| RestError::not_found(format!("Block {height} not found")))? + rest.ledger.get_block(height).map_err(|_| RestError::not_found(format!("Block {height} not found")))? } else { - let hash = height_or_hash - .parse::() - .map_err(|_| RestError::bad_request("invalid input, it is neither a block height nor a block hash"))?; + let hash = height_or_hash.parse::().map_err(|_| { + RestError::bad_request("invalid input, it is neither a block height nor a block hash".to_string()) + })?; - rest - .ledger - .get_block_by_hash(&hash) - .map_err(|_| RestError::not_found(format!("Block {hash} not found")))? + rest.ledger.get_block_by_hash(&hash).map_err(|_| RestError::not_found(format!("Block {hash} not found")))? }; Ok(ErasedJson::pretty(block)) @@ -142,7 +136,7 @@ impl, R: Routing> Rest { // Ensure the end height is greater than the start height. if start_height > end_height { - return Err(RestError::bad_request("Invalid block range")); + return Err(RestError::bad_request("Invalid block range".to_string())); } // Ensure the block range is bounded. @@ -156,7 +150,9 @@ impl, R: Routing> Rest { // Prepare a closure for the blocking work. let get_json_blocks = move || -> Result { let blocks = cfg_into_iter!(start_height..end_height) - .map(|height| rest.ledger.get_block(height).map_err(|_| RestError::not_found(format!("Block {height} not found")))) + .map(|height| { + rest.ledger.get_block(height).map_err(|_| RestError::not_found(format!("Block {height} not found"))) + }) .collect::, _>>()?; Ok(ErasedJson::pretty(blocks)) @@ -165,9 +161,9 @@ impl, R: Routing> Rest { // Fetch the blocks from ledger and serialize to json. match tokio::task::spawn_blocking(get_json_blocks).await { Ok(json) => json, - Err(err) => Err(RestError::not_found(format!( - "Failed to get blocks '{start_height}..{end_height}' - {err}" - ))), + Err(err) => { + Err(RestError::not_found(format!("Failed to get blocks '{start_height}..{end_height}' - {err}"))) + } } } @@ -292,7 +288,7 @@ impl, R: Routing> Rest { Some(consensus) => { Ok(ErasedJson::pretty(consensus.unconfirmed_transmissions().collect::>())) } - None => Err(RestError::not_found("Route isn't available for this node type")), + None => Err(RestError::not_found("Route isn't available for this node type".to_string())), } } @@ -300,7 +296,7 @@ impl, R: Routing> Rest { pub(crate) async fn get_memory_pool_solutions(State(rest): State) -> Result { match rest.consensus { Some(consensus) => Ok(ErasedJson::pretty(consensus.unconfirmed_solutions().collect::>())), - None => Err(RestError::not_found("Route isn't available for this node type")), + None => Err(RestError::not_found("Route isn't available for this node type".to_string())), } } @@ -308,7 +304,7 @@ impl, R: Routing> Rest { pub(crate) async fn get_memory_pool_transactions(State(rest): State) -> Result { match rest.consensus { Some(consensus) => Ok(ErasedJson::pretty(consensus.unconfirmed_transactions().collect::>())), - None => Err(RestError::not_found("Route isn't available for this node type")), + None => Err(RestError::not_found("Route isn't available for this node type".to_string())), } } @@ -421,7 +417,7 @@ impl, R: Routing> Rest { // Return an error if the `all` query parameter is not set to `true`. if metadata.all != Some(true) { return Err(RestError::bad_request( - "Invalid query parameter. At this time, 'all=true' must be included", + "Invalid query parameter. At this time, 'all=true' must be included".to_string(), )); } @@ -490,20 +486,14 @@ impl, R: Routing> Rest { ) -> Result { // Do not process the request if the node is too far behind to avoid sending outdated data. if !rest.routing.is_within_sync_leniency() { - return Err(RestError::service_unavailable( - "Unable to request delegators (node is syncing)", - )); + return Err(RestError::service_unavailable("Unable to request delegators (node is syncing)".to_string())); } // Return the delegators for the given validator. match tokio::task::spawn_blocking(move || rest.ledger.get_delegators_for_validator(&validator)).await { Ok(Ok(delegators)) => Ok(ErasedJson::pretty(delegators)), - Ok(Err(err)) => Err(RestError::service_unavailable(format!( - "Unable to request delegators - {err}" - ))), - Err(err) => Err(RestError::service_unavailable(format!( - "Unable to request delegators - {err}" - ))), + Ok(Err(err)) => Err(RestError::service_unavailable(format!("Unable to request delegators - {err}"))), + Err(err) => Err(RestError::service_unavailable(format!("Unable to request delegators - {err}"))), } } @@ -596,7 +586,7 @@ impl, R: Routing> Rest { // TODO: Should this be a blocking task? let buffer = Vec::with_capacity(3000); if tx.write_le(LimitedWriter::new(buffer, N::MAX_TRANSACTION_SIZE)).is_err() { - return Err(RestError::bad_request("Transaction size exceeds the byte limit")); + return Err(RestError::bad_request("Transaction size exceeds the byte limit".to_string())); } if check_transaction.check_transaction.unwrap_or(false) { @@ -774,7 +764,7 @@ impl, R: Routing> Rest { Ok(ErasedJson::pretty(participation_scores)) } - None => Err(RestError::not_found("Route isn't available for this node type")), + None => Err(RestError::not_found("Route isn't available for this node type".to_string())), } } } From 1539485ea24966d1a92e3c4aa2156efc4b3ed446 Mon Sep 17 00:00:00 2001 From: Damon Sicore Date: Tue, 19 Aug 2025 18:17:06 -0500 Subject: [PATCH 3/3] Fix delegator sync error message --- node/rest/src/routes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/rest/src/routes.rs b/node/rest/src/routes.rs index d95cb18832..fe6f20176f 100644 --- a/node/rest/src/routes.rs +++ b/node/rest/src/routes.rs @@ -486,7 +486,7 @@ impl, R: Routing> Rest { ) -> Result { // Do not process the request if the node is too far behind to avoid sending outdated data. if !rest.routing.is_within_sync_leniency() { - return Err(RestError::service_unavailable("Unable to request delegators (node is syncing)".to_string())); + return Err(RestError::service_unavailable("Unable to request delegators (node is syncing)".to_string())); } // Return the delegators for the given validator.