From d9a170ee03b574018b23d860552d3d184df49a2f Mon Sep 17 00:00:00 2001 From: Daniel Porteous Date: Thu, 21 Jul 2022 17:37:43 -0700 Subject: [PATCH] [API] Rework response and error handling --- Cargo.lock | 1 + api/Cargo.toml | 1 + api/src/context.rs | 54 ++-- api/src/failpoint.rs | 9 +- api/src/poem_backend/accept_type.rs | 40 ++- api/src/poem_backend/accounts.rs | 188 ++++++------ api/src/poem_backend/events.rs | 68 ++--- api/src/poem_backend/index.rs | 22 +- api/src/poem_backend/mod.rs | 6 +- api/src/poem_backend/page.rs | 46 ++- api/src/poem_backend/response.rs | 419 +++++++++++++++++++-------- api/src/poem_backend/transactions.rs | 77 +++-- api/types/src/address.rs | 6 - 13 files changed, 546 insertions(+), 391 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e779516f0278..e074c4fa5782f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -224,6 +224,7 @@ dependencies = [ "mime", "move-deps", "once_cell", + "paste", "percent-encoding", "poem", "poem-openapi", diff --git a/api/Cargo.toml b/api/Cargo.toml index 4f09a10c5023d..e0cb5273d4da3 100644 --- a/api/Cargo.toml +++ b/api/Cargo.toml @@ -19,6 +19,7 @@ hex = "0.4.3" hyper = "0.14.18" mime = "0.3.16" once_cell = "1.10.0" +paste = "1.0.7" percent-encoding = "2.1.0" poem = { version = "1.3.35", features = ["anyhow", "rustls"] } poem-openapi = { version = "2.0.5", features = ["swagger-ui", "url"] } diff --git a/api/src/context.rs b/api/src/context.rs index ea0cddc6e203f..58cccdf94c82f 100644 --- a/api/src/context.rs +++ b/api/src/context.rs @@ -1,7 +1,7 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use anyhow::{anyhow, ensure, format_err, Result}; +use anyhow::{anyhow, ensure, format_err, Context as AnyhowContext, Result}; use aptos_api_types::{AsConverter, BlockInfo, Error, LedgerInfo, TransactionOnChainData, U64}; use aptos_config::config::{NodeConfig, RoleType}; use aptos_crypto::HashValue; @@ -23,7 +23,6 @@ use aptos_types::{ use aptos_vm::data_cache::{IntoMoveResolver, RemoteStorageOwned}; use futures::{channel::oneshot, SinkExt}; use move_deps::move_core_types::ident_str; -use poem_openapi::payload::Json; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, convert::Infallible, sync::Arc}; use storage_interface::{ @@ -32,7 +31,7 @@ use storage_interface::{ }; use warp::{filters::BoxedFilter, Filter, Reply}; -use crate::poem_backend::{AptosError, AptosErrorCode, AptosErrorResponse, AptosInternalResult}; +use crate::poem_backend::{AptosErrorCode, InternalError}; // Context holds application scope context #[derive(Clone)] @@ -64,16 +63,12 @@ impl Context { .map(|state_view| state_view.into_move_resolver()) } - pub fn move_resolver_poem(&self) -> AptosInternalResult> { - self.move_resolver().map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to read latest state checkpoint from DB: {}", e) - .to_string(), - ) - .error_code(AptosErrorCode::ReadFromStorageError), - )) - }) + pub fn move_resolver_poem( + &self, + ) -> Result, E> { + self.move_resolver() + .context("Failed to read latest state checkpoint from DB") + .map_err(|e| E::internal(e).error_code(AptosErrorCode::ReadFromStorageError)) } pub fn state_view_at_version(&self, version: Version) -> Result { @@ -118,13 +113,21 @@ impl Context { } } - pub fn get_latest_ledger_info_poem(&self) -> AptosInternalResult { - self.get_latest_ledger_info().map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new(format_err!("Failed to retrieve ledger info: {}", e).to_string()) - .error_code(AptosErrorCode::ReadFromStorageError), + // TODO: Add error codes to these errors. + pub fn get_latest_ledger_info_poem(&self) -> Result { + if let Some(oldest_version) = self.db.get_first_txn_version().map_err(E::internal)? { + Ok(LedgerInfo::new( + &self.chain_id(), + &self + .get_latest_ledger_info_with_signatures() + .map_err(E::internal)?, + oldest_version, )) - }) + } else { + Err(E::internal(anyhow!( + "Failed to retrieve latest ledger info" + ))) + } } pub fn get_latest_ledger_info_with_signatures(&self) -> Result { @@ -137,17 +140,14 @@ impl Context { .get_state_value(state_key) } - pub fn get_state_value_poem( + pub fn get_state_value_poem( &self, state_key: &StateKey, version: u64, - ) -> Result>, AptosErrorResponse> { - self.get_state_value(state_key, version).map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new(format_err!("Failed to retrieve state value: {}", e).to_string()) - .error_code(AptosErrorCode::ReadFromStorageError), - )) - }) + ) -> Result>, E> { + self.get_state_value(state_key, version) + .context("Failed to retrieve state value") + .map_err(|e| E::internal(e).error_code(AptosErrorCode::ReadFromStorageError)) } pub fn get_state_values( diff --git a/api/src/failpoint.rs b/api/src/failpoint.rs index de38cc097f43d..6fca4f11fc616 100644 --- a/api/src/failpoint.rs +++ b/api/src/failpoint.rs @@ -6,7 +6,7 @@ use anyhow::{format_err, Result}; use aptos_api_types::Error; -use crate::poem_backend::{AptosError, AptosErrorResponse}; +use crate::poem_backend::{AptosError, InternalError}; use poem_openapi::payload::Json; #[allow(unused_variables)] @@ -19,10 +19,11 @@ pub fn fail_point(name: &str) -> Result<(), Error> { #[allow(unused_variables)] #[inline] -pub fn fail_point_poem(name: &str) -> Result<(), AptosErrorResponse> { +pub fn fail_point_poem(name: &str) -> Result<(), E> { Ok(fail::fail_point!(format!("api::{}", name).as_str(), |_| { - Err(AptosErrorResponse::InternalServerError(Json( - AptosError::new(format!("unexpected internal error for {}", name)), + Err(E::internal_str(&format!( + "unexpected internal error for {}", + name ))) })) } diff --git a/api/src/poem_backend/accept_type.rs b/api/src/poem_backend/accept_type.rs index 98e987a0cd79c..0fe8df21b6c77 100644 --- a/api/src/poem_backend/accept_type.rs +++ b/api/src/poem_backend/accept_type.rs @@ -1,12 +1,9 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use std::convert::TryFrom; - use poem::web::Accept; -use poem_openapi::payload::Json; -use super::{AptosError, AptosErrorCode, AptosErrorResponse}; +use super::{AptosErrorCode, BadRequestError}; #[derive(PartialEq)] pub enum AcceptType { @@ -14,25 +11,24 @@ pub enum AcceptType { Bcs, } -impl TryFrom<&Accept> for AcceptType { - type Error = AptosErrorResponse; - - fn try_from(accept: &Accept) -> Result { - for mime in &accept.0 { - match mime.as_ref() { - "application/json" => return Ok(AcceptType::Json), - "application/x-bcs" => return Ok(AcceptType::Bcs), - "*/*" => {} - wildcard => { - return Err(AptosErrorResponse::BadRequest(Json( - AptosError::new(format!("Invalid Accept type: {:?}", wildcard)) - .error_code(AptosErrorCode::UnsupportedAcceptType), - ))); - } +// I can't use TryFrom here right now: +// https://stackoverflow.com/questions/73072492/apply-trait-bounds-to-associated-type +pub fn parse_accept(accept: &Accept) -> Result { + for mime in &accept.0 { + match mime.as_ref() { + "application/json" => return Ok(AcceptType::Json), + "application/x-bcs" => return Ok(AcceptType::Bcs), + "*/*" => {} + wildcard => { + return Err(E::bad_request_str(&format!( + "Unsupported Accept type: {:?}", + wildcard + )) + .error_code(AptosErrorCode::UnsupportedAcceptType)); } } - - // Default to returning content as JSON. - Ok(AcceptType::Json) } + + // Default to returning content as JSON. + Ok(AcceptType::Json) } diff --git a/api/src/poem_backend/accounts.rs b/api/src/poem_backend/accounts.rs index bd7ae98f0365e..e6ba202350119 100644 --- a/api/src/poem_backend/accounts.rs +++ b/api/src/poem_backend/accounts.rs @@ -1,19 +1,19 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use std::convert::{TryFrom, TryInto}; +use anyhow::Context as AnyhowContext; +use std::convert::TryInto; +use std::fmt::Display; use std::sync::Arc; -use super::accept_type::AcceptType; -use super::response::deserialize_from_bcs; +use super::accept_type::{parse_accept, AcceptType}; use super::{ - response::{AptosInternalResult, AptosResponseResult}, - ApiTags, AptosResponse, + ApiTags, AptosErrorResponse, BadRequestError, BasicResponse, BasicResponseStatus, + InternalError, NotFoundError, }; -use super::{AptosError, AptosErrorCode, AptosErrorResponse}; +use super::{AptosErrorCode, BasicErrorWith404, BasicResultWith404}; use crate::context::Context; use crate::failpoint::fail_point_poem; -use anyhow::format_err; use aptos_api_types::{AccountData, Address, AsConverter, MoveStructTag, TransactionId}; use aptos_api_types::{LedgerInfo, MoveModuleBytecode, MoveResource}; use aptos_types::access_path::AccessPath; @@ -30,12 +30,8 @@ use move_deps::move_core_types::{ }; use poem::web::Accept; use poem_openapi::param::Query; -use poem_openapi::payload::Json; use poem_openapi::{param::Path, OpenApi}; -// TODO: Make a helper that builds an AptosResponse from just an anyhow error, -// that assumes that it's an internal error. We can use .context() add more info. - pub struct AccountsApi { pub context: Arc, } @@ -56,9 +52,9 @@ impl AccountsApi { accept: Accept, address: Path
, ledger_version: Query>, - ) -> AptosResponseResult { - fail_point_poem("endpoint_get_account")?; - let accept_type = AcceptType::try_from(&accept)?; + ) -> BasicResultWith404 { + fail_point_poem::("endpoint_get_account")?; + let accept_type = parse_accept::(&accept)?; let account = Account::new(self.context.clone(), address.0, ledger_version.0)?; account.account(&accept_type) } @@ -80,9 +76,9 @@ impl AccountsApi { accept: Accept, address: Path
, ledger_version: Query>, - ) -> AptosResponseResult> { - fail_point_poem("endpoint_get_account_resources")?; - let accept_type = AcceptType::try_from(&accept)?; + ) -> BasicResultWith404> { + fail_point_poem::("endpoint_get_account_resources")?; + let accept_type = parse_accept::(&accept)?; let account = Account::new(self.context.clone(), address.0, ledger_version.0)?; account.resources(&accept_type) } @@ -104,9 +100,9 @@ impl AccountsApi { accept: Accept, address: Path
, ledger_version: Query>, - ) -> AptosResponseResult> { - fail_point_poem("endpoint_get_account_resources")?; - let accept_type = AcceptType::try_from(&accept)?; + ) -> BasicResultWith404> { + fail_point_poem::("endpoint_get_account_modules")?; + let accept_type = parse_accept::(&accept)?; let account = Account::new(self.context.clone(), address.0, ledger_version.0)?; account.modules(&accept_type) } @@ -124,13 +120,13 @@ impl Account { context: Arc, address: Address, requested_ledger_version: Option, - ) -> Result { - let latest_ledger_info = context.get_latest_ledger_info_poem()?; + ) -> Result { + let latest_ledger_info = context.get_latest_ledger_info_poem::()?; let ledger_version: u64 = requested_ledger_version.unwrap_or_else(|| latest_ledger_info.version()); if ledger_version > latest_ledger_info.version() { - return Err(AptosErrorResponse::not_found( + return Err(Self::not_found( "ledger", TransactionId::Version(ledger_version), latest_ledger_info.version(), @@ -147,7 +143,7 @@ impl Account { // These functions map directly to endpoint functions. - pub fn account(self, accept_type: &AcceptType) -> AptosResponseResult { + pub fn account(self, accept_type: &AcceptType) -> BasicResultWith404 { let state_key = StateKey::AccessPath(AccessPath::resource_access_path(ResourceKey::new( self.address.into(), AccountResource::struct_tag(), @@ -155,90 +151,90 @@ impl Account { let state_value = self .context - .get_state_value_poem(&state_key, self.ledger_version)?; + .get_state_value_poem::(&state_key, self.ledger_version)?; let state_value = match state_value { Some(state_value) => state_value, None => return Err(self.resource_not_found(&AccountResource::struct_tag())), }; - let account_resource = deserialize_from_bcs::(&state_value)?; + let account_resource: AccountResource = bcs::from_bytes(&state_value) + .context("Internal error deserializing response from DB") + .map_err(BasicErrorWith404::internal)?; let account_data: AccountData = account_resource.into(); - AptosResponse::try_from_rust_value(account_data, &self.latest_ledger_info, accept_type) + BasicResponse::try_from_rust_value(( + account_data, + &self.latest_ledger_info, + BasicResponseStatus::Ok, + accept_type, + )) } - pub fn resources(self, accept_type: &AcceptType) -> AptosResponseResult> { + pub fn resources(self, accept_type: &AcceptType) -> BasicResultWith404> { let account_state = self.account_state()?; let resources = account_state.get_resources(); - let move_resolver = self.context.move_resolver_poem()?; + let move_resolver = self.context.move_resolver_poem::()?; let converted_resources = move_resolver .as_converter() .try_into_resources(resources) - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!( - "Failed to build move resource response from data in DB: {}", - e - ) - .to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - })?; - AptosResponse::>::try_from_rust_value( + .context("Failed to build move resource response from data in DB") + .map_err(BasicErrorWith404::internal) + .map_err(|e| e.error_code(AptosErrorCode::InvalidBcsInStorageError))?; + + BasicResponse::try_from_rust_value(( converted_resources, &self.latest_ledger_info, + BasicResponseStatus::Ok, accept_type, - ) + )) } - pub fn modules(self, accept_type: &AcceptType) -> AptosResponseResult> { + pub fn modules(self, accept_type: &AcceptType) -> BasicResultWith404> { let mut modules = Vec::new(); for module in self.account_state()?.into_modules() { modules.push( MoveModuleBytecode::new(module) .try_parse_abi() - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to parse move module ABI: {}", e).to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - })?, + .context("Failed to parse move module ABI") + .map_err(BasicErrorWith404::internal) + .map_err(|e| e.error_code(AptosErrorCode::InvalidBcsInStorageError))?, ); } - AptosResponse::>::try_from_rust_value( + BasicResponse::try_from_rust_value(( modules, &self.latest_ledger_info, + BasicResponseStatus::Ok, accept_type, - ) + )) } // Helpers for processing account state. - fn account_state(&self) -> AptosInternalResult { + fn account_state(&self) -> Result { let state = self .context .get_account_state(self.address.into(), self.ledger_version) - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to read account state from DB: {}", e).to_string(), - ) - .error_code(AptosErrorCode::ReadFromStorageError), - )) - })? + .map_err(BasicErrorWith404::internal) + .map_err(|e| e.error_code(AptosErrorCode::ReadFromStorageError))? .ok_or_else(|| self.account_not_found())?; + Ok(state) } // Helpers for building errors. - fn account_not_found(&self) -> AptosErrorResponse { - AptosErrorResponse::not_found( + pub fn not_found( + resource: &str, + identifier: S, + ledger_version: u64, + ) -> BasicErrorWith404 { + BasicErrorWith404::not_found_str(&format!("{} not found by {}", resource, identifier)) + .aptos_ledger_version(ledger_version) + } + + fn account_not_found(&self) -> BasicErrorWith404 { + Self::not_found( "account", format!( "address({}) and ledger version({})", @@ -248,8 +244,8 @@ impl Account { ) } - fn resource_not_found(&self, struct_tag: &StructTag) -> AptosErrorResponse { - AptosErrorResponse::not_found( + fn resource_not_found(&self, struct_tag: &StructTag) -> BasicErrorWith404 { + Self::not_found( "resource", format!( "address({}), struct tag({}) and ledger version({})", @@ -263,8 +259,8 @@ impl Account { &self, struct_tag: &StructTag, field_name: &Identifier, - ) -> AptosErrorResponse { - AptosErrorResponse::not_found( + ) -> BasicErrorWith404 { + Self::not_found( "resource", format!( "address({}), struct tag({}), field name({}) and ledger version({})", @@ -283,12 +279,11 @@ impl Account { &self, event_handle: MoveStructTag, field_name: Identifier, - ) -> AptosInternalResult { - let struct_tag: StructTag = event_handle.try_into().map_err(|e| { - AptosErrorResponse::BadRequest(Json(AptosError::new( - format_err!("Given event handle was invalid: {}", e).to_string(), - ))) - })?; + ) -> Result { + let struct_tag: StructTag = event_handle + .try_into() + .context("Given event handle was invalid") + .map_err(BasicErrorWith404::bad_request)?; let resource = self.find_resource(&struct_tag)?; @@ -297,44 +292,35 @@ impl Account { .find(|(id, _)| id == &field_name) .ok_or_else(|| self.field_not_found(&struct_tag, &field_name))?; - // serialization should not fail, otherwise it's internal bug - // TODO: don't unwrap - let event_handle_bytes = bcs::to_bytes(&value).unwrap(); - // deserialization may fail because the bytes are not EventHandle struct type. - let event_handle: EventHandle = bcs::from_bytes(&event_handle_bytes).map_err(|e| { - AptosErrorResponse::BadRequest(Json(AptosError::new( - format_err!( - "Deserialization error, field({}) type is not EventHandle struct: {}", - field_name, - e - ) - .to_string(), - ))) - })?; + // Serialization should not fail, otherwise it's internal bug + let event_handle_bytes = bcs::to_bytes(&value) + .context("Failed to serialize event handle, this is an internal bug") + .map_err(BasicErrorWith404::internal)?; + // Deserialization may fail because the bytes are not EventHandle struct type. + let event_handle: EventHandle = bcs::from_bytes(&event_handle_bytes) + .context(format!( + "Deserialization error, field({}) type is not EventHandle struct", + field_name + )) + .map_err(BasicErrorWith404::bad_request)?; Ok(*event_handle.key()) } fn find_resource( &self, struct_tag: &StructTag, - ) -> AptosInternalResult> { + ) -> Result, BasicErrorWith404> { let account_state = self.account_state()?; let (typ, data) = account_state .get_resources() .find(|(tag, _data)| tag == struct_tag) .ok_or_else(|| self.resource_not_found(struct_tag))?; - let move_resolver = self.context.move_resolver_poem()?; + let move_resolver = self.context.move_resolver_poem::()?; move_resolver .as_converter() .move_struct_fields(&typ, data) - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to convert move structs: {}", e).to_string(), - ) - .error_code(AptosErrorCode::ReadFromStorageError), - )) - }) + .context("Failed to convert move structs") + .map_err(BasicErrorWith404::internal) } } @@ -347,14 +333,14 @@ impl Account { .get_state_values(self.address.into(), self.ledger_version) .ok_or_else(|| self.account_not_found())?; match accept_type { - AcceptType::Bcs => Ok(AptosResponse::from_bcs( + AcceptType::Bcs => Ok(BasicResponse::from_bcs( state_value, &self.latest_ledger_info, )), AcceptType::Json => { let account_resource = deserialize_from_bcs::(&state_value)?; let account_data: AccountData = account_resource.into(); - Ok(AptosResponse::from_json( + Ok(BasicResponse::from_json( account_data, &self.latest_ledger_info, )) diff --git a/api/src/poem_backend/events.rs b/api/src/poem_backend/events.rs index af954b4ee83e7..f32000065f500 100644 --- a/api/src/poem_backend/events.rs +++ b/api/src/poem_backend/events.rs @@ -1,27 +1,24 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use std::convert::TryFrom; use std::sync::Arc; -use super::accept_type::AcceptType; +use super::accept_type::{parse_accept, AcceptType}; use super::accounts::Account; use super::page::Page; -use super::{response::AptosResponseResult, ApiTags, AptosResponse}; -use super::{AptosError, AptosErrorCode, AptosErrorResponse}; +use super::{ + ApiTags, BadRequestError, BasicErrorWith404, BasicResponse, BasicResponseStatus, + BasicResultWith404, InternalError, +}; use crate::context::Context; use crate::failpoint::fail_point_poem; -use anyhow::format_err; +use anyhow::Context as AnyhowContext; use aptos_api_types::{Address, EventKey, IdentifierWrapper, MoveStructTagWrapper}; use aptos_api_types::{AsConverter, Event}; use poem::web::Accept; use poem_openapi::param::Query; -use poem_openapi::payload::Json; use poem_openapi::{param::Path, OpenApi}; -// TODO: Make a helper that builds an AptosResponse from just an anyhow error, -// that assumes that it's an internal error. We can use .context() add more info. - pub struct EventsApi { pub context: Arc, } @@ -46,9 +43,9 @@ impl EventsApi { event_key: Path, start: Query>, limit: Query>, - ) -> AptosResponseResult> { - fail_point_poem("endpoint_get_events_by_event_key")?; - let accept_type = AcceptType::try_from(&accept)?; + ) -> BasicResultWith404> { + fail_point_poem::("endpoint_get_events_by_event_key")?; + let accept_type = parse_accept::(&accept)?; let page = Page::new(start.0, limit.0); self.list(&accept_type, page, event_key.0) } @@ -72,9 +69,9 @@ impl EventsApi { field_name: Path, start: Query>, limit: Query>, - ) -> AptosResponseResult> { - fail_point_poem("endpoint_get_events_by_event_handle")?; - let accept_type = AcceptType::try_from(&accept)?; + ) -> BasicResultWith404> { + fail_point_poem::("endpoint_get_events_by_event_handle")?; + let accept_type = parse_accept::(&accept)?; let page = Page::new(start.0, limit.0); let account = Account::new(self.context.clone(), address.0, None)?; let key = account @@ -90,42 +87,35 @@ impl EventsApi { accept_type: &AcceptType, page: Page, event_key: EventKey, - ) -> AptosResponseResult> { - let latest_ledger_info = self.context.get_latest_ledger_info_poem()?; + ) -> BasicResultWith404> { + let latest_ledger_info = self + .context + .get_latest_ledger_info_poem::()?; let contract_events = self .context .get_events( &event_key.into(), - page.start(0, u64::MAX)?, - page.limit()?, + page.start::(0, u64::MAX)?, + page.limit::()?, latest_ledger_info.version(), ) // TODO: Previously this was a 500, but I'm making this a 400. I suspect // both could be true depending on the error. Make this more specific. - .map_err(|e| { - AptosErrorResponse::BadRequest(Json( - AptosError::new( - format_err!("Failed to find events by key {}: {}", event_key, e) - .to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - })?; + .context(format!("Failed to find events by key {}", event_key)) + .map_err(BasicErrorWith404::bad_request)?; - let resolver = self.context.move_resolver_poem()?; + let resolver = self.context.move_resolver_poem::()?; let events = resolver .as_converter() .try_into_events(&contract_events) - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to convert events from storage into response: {}", e) - .to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - })?; + .context("Failed to convert events from storage into response {}") + .map_err(BasicErrorWith404::internal)?; - AptosResponse::try_from_rust_value(events, &latest_ledger_info, accept_type) + BasicResponse::try_from_rust_value(( + events, + &latest_ledger_info, + BasicResponseStatus::Ok, + accept_type, + )) } } diff --git a/api/src/poem_backend/index.rs b/api/src/poem_backend/index.rs index 8c0cf0a54f71f..a93c4a6aff5d6 100644 --- a/api/src/poem_backend/index.rs +++ b/api/src/poem_backend/index.rs @@ -1,10 +1,11 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use std::{convert::TryFrom, sync::Arc}; +use std::sync::Arc; -use super::accept_type::AcceptType; -use super::{response::AptosResponseResult, ApiTags, AptosResponse}; +use super::accept_type::parse_accept; +use super::ApiTags; +use super::{BasicError, BasicResponse, BasicResponseStatus, BasicResult}; use crate::context::Context; use aptos_api_types::IndexResponse; use poem::web::Accept; @@ -25,11 +26,18 @@ impl IndexApi { operation_id = "get_ledger_info", tag = "ApiTags::General" )] - async fn get_ledger_info(&self, accept: Accept) -> AptosResponseResult { - let accept_type = AcceptType::try_from(&accept)?; - let ledger_info = self.context.get_latest_ledger_info_poem()?; + async fn get_ledger_info(&self, accept: Accept) -> BasicResult { + let accept_type = parse_accept::(&accept)?; + let ledger_info = self.context.get_latest_ledger_info_poem::()?; + let node_role = self.context.node_role(); let index_response = IndexResponse::new(ledger_info.clone(), node_role); - AptosResponse::try_from_rust_value(index_response, &ledger_info, &accept_type) + + BasicResponse::try_from_rust_value::(( + index_response, + &ledger_info, + BasicResponseStatus::Ok, + &accept_type, + )) } } diff --git a/api/src/poem_backend/mod.rs b/api/src/poem_backend/mod.rs index 0af6b33227df5..4790e8cd048b2 100644 --- a/api/src/poem_backend/mod.rs +++ b/api/src/poem_backend/mod.rs @@ -22,16 +22,14 @@ pub enum ApiTags { General, } +pub use accept_type::AcceptType; pub use accounts::AccountsApi; pub use basic::BasicApi; pub use events::EventsApi; pub use index::IndexApi; pub use log::middleware_log; pub use post::AptosPost; -pub use response::{ - AptosError, AptosErrorCode, AptosErrorResponse, AptosInternalResult, AptosResponse, - AptosResponseContent, -}; +pub use response::*; pub use runtime::attach_poem_to_runtime; pub use transactions::TransactionsApi; diff --git a/api/src/poem_backend/page.rs b/api/src/poem_backend/page.rs index bb5a5eb25ad77..4d56539fdc939 100644 --- a/api/src/poem_backend/page.rs +++ b/api/src/poem_backend/page.rs @@ -1,8 +1,7 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use super::{response::AptosInternalResult, AptosError, AptosErrorCode, AptosErrorResponse}; -use poem_openapi::payload::Json; +use super::{AptosErrorCode, BadRequestError}; use serde::Deserialize; const DEFAULT_PAGE_SIZE: u16 = 25; @@ -18,43 +17,34 @@ impl Page { pub fn new(start: Option, limit: Option) -> Self { Self { start, limit } } -} -impl Page { - pub fn start(&self, default: u64, max: u64) -> AptosInternalResult { + pub fn start(&self, default: u64, max: u64) -> Result { let start = self.start.unwrap_or(default); if start > max { - return Err(AptosErrorResponse::BadRequest(Json( - AptosError::new( - anyhow::format_err!( - "Given start value ({}) is too large, it must be < {}", - start, - max - ) - .to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - ))); + return Err(E::bad_request_str(&format!( + "Given start value ({}) is too large, it must be < {}", + start, max + )) + .error_code(AptosErrorCode::InvalidStartParam)); } Ok(start) } - pub fn limit(&self) -> AptosInternalResult { + pub fn limit(&self) -> Result { let limit = self.limit.unwrap_or(DEFAULT_PAGE_SIZE); if limit == 0 { - return Err(AptosErrorResponse::BadRequest(Json(AptosError::new( - anyhow::format_err!("Given limit value ({}) must not be zero", limit,).to_string(), - )))); + return Err(E::bad_request_str(&format!( + "Given limit value ({}) must not be zero", + limit + )) + .error_code(AptosErrorCode::InvalidLimitParam)); } if limit > MAX_PAGE_SIZE { - return Err(AptosErrorResponse::BadRequest(Json(AptosError::new( - anyhow::format_err!( - "Given limit value ({}) is too large, it must be < {}", - limit, - MAX_PAGE_SIZE - ) - .to_string(), - )))); + return Err(E::bad_request_str(&format!( + "Given limit value ({}) is too large, it must be < {}", + limit, MAX_PAGE_SIZE + )) + .error_code(AptosErrorCode::InvalidLimitParam)); } Ok(limit) } diff --git a/api/src/poem_backend/response.rs b/api/src/poem_backend/response.rs index 8ebb0a81fdaf2..e38deb5080a7c 100644 --- a/api/src/poem_backend/response.rs +++ b/api/src/poem_backend/response.rs @@ -1,26 +1,40 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use std::fmt::Display; +//! The Aptos API response / error handling philosophy. +//! +//! The return type for every endpoint should be a +//! poem::Result, MyError> where MyResponse is an instance of +//! ApiResponse that contains only the status codes that it can actually +//! return. This will manifest in the OpenAPI spec, making it clear to users +//! what the API can actually return. The error should operate the same way, +//! where it only describes error codes that it can actually return. +//! +//! Every endpoint should be able to return data as JSON or BCS, depending on +//! the Accept header given by the client. If none is given, default to JSON. +//! +//! The client should be able to provide data to POST endpoints as either JSON +//! or BCS, given they provide the appropriate Content-Type header. +//! +//! Where possible, if the API is returning data as BCS, it should pull the +//! bytes directly from the DB where possible without further processing. +//! +//! Internally, there are many functions that can return errors. The context +//! for what type of error those functions return is lost if we return an +//! opaque error type like anyhow::Error. As such, it is important that each +//! function return error types that capture the intended status code. This +//! module defines traits to help with this, ensuring that the error types +//! returned by each function and its callers is enforced at compile time. +//! See generate_error_traits and its invocations for more on this topic. + +// TODO: Pending further discussion with the team, migrate back to U64 over u64. use super::accept_type::AcceptType; -use anyhow::format_err; -use aptos_api_types::{LedgerInfo, U64}; -use poem::Result as PoemResult; -use poem_openapi::{payload::Json, types::ToJSON, ApiResponse, Enum, Object, ResponseContent}; -use serde::{Deserialize, Serialize}; +use aptos_api_types::U64; +use poem_openapi::{payload::Json, types::ToJSON, Enum, Object, ResponseContent}; use super::bcs_payload::Bcs; -// This should be used for endpoints, signalling that they return either a -// response capturing success or failure. -pub type AptosResponseResult = PoemResult, AptosErrorResponse>; - -// This should be used for internal functions that need to return just a T -// but could fail, in which case we bubble an error response up to the client. -pub type AptosInternalResult = anyhow::Result; - -// TODO: Consdider having more specific error structs for different endpoints. /// This is the generic struct we use for all API errors, it contains a string /// message and an Aptos API specific error code. #[derive(Debug, Object)] @@ -49,6 +63,12 @@ impl AptosError { } } +impl From for AptosError { + fn from(error: anyhow::Error) -> Self { + AptosError::new(error.to_string()) + } +} + /// These codes provide more granular error information beyond just the HTTP /// status code of the response. // Make sure the integer codes increment one by one. @@ -66,19 +86,12 @@ pub enum AptosErrorCode { /// We were unexpectedly unable to convert a Rust type to BCS. BcsSerializationError = 3, -} - -// TODO: Find a less repetitive way to do this. -#[derive(ApiResponse)] -pub enum AptosErrorResponse { - #[oai(status = 400)] - BadRequest(Json), - #[oai(status = 404)] - NotFound(Json), + /// The start param given for paging is invalid. + InvalidStartParam = 4, - #[oai(status = 500)] - InternalServerError(Json), + /// The limit param given for paging is invalid. + InvalidLimitParam = 5, } #[derive(ResponseContent)] @@ -95,105 +108,285 @@ pub enum AptosResponseContent { Bcs(Bcs>), } -#[derive(ApiResponse)] -pub enum AptosResponse { - #[oai(status = 200)] - Ok( - AptosResponseContent, - #[oai(header = "X-Aptos-Chain-Id")] u16, - #[oai(header = "X-Aptos-Ledger-Version")] u64, - #[oai(header = "X-Aptos-Ledger-Oldest-Version")] u64, - #[oai(header = "X-Aptos-Ledger-TimestampUsec")] u64, - #[oai(header = "X-Aptos-Epoch")] u64, - ), -} - -// From impls +/// This trait defines common functions that all error responses should impl. +/// As a user you shouldn't worry about this, the generate_error_response macro +/// takes care of it for you. Mostly these are helpers to allow callers holding +/// an error response to manipulate the AptosError inside it. +pub trait AptosErrorResponse { + fn inner_mut(&mut self) -> &mut AptosError; -impl From for AptosError { - fn from(error: anyhow::Error) -> Self { - AptosError::new(error.to_string()) + fn error_code(mut self, error_code: AptosErrorCode) -> Self + where + Self: Sized, + { + self.inner_mut().error_code = Some(error_code); + self } -} -impl AptosErrorResponse { - pub fn not_found(resource: &str, identifier: S, ledger_version: u64) -> Self { - Self::NotFound(Json( - AptosError::new(format!("{} not found by {}", resource, identifier)) - .aptos_ledger_version(ledger_version), - )) + fn aptos_ledger_version(mut self, aptos_ledger_version: u64) -> Self + where + Self: Sized, + { + self.inner_mut().aptos_ledger_version = Some(aptos_ledger_version.into()); + self } +} - pub fn invalid_param(name: &str, value: S) -> Self { - Self::BadRequest(Json(AptosError::new(format!( - "invalid parameter {}: {}", - name, value - )))) - } +/// This macro defines traits for all of the given status codes. In eahc trait +/// there is a function that defines a helper for building an instance of the +/// error type using that code. These traits are helpful for defining what +/// error types an internal function can return. For example, the failpoint +/// function can return an internal error, so its signature would look like: +/// fn fail_point_poem(name: &str) -> anyhow::Result<(), E> +/// This should be invoked just once, taking in every status that we use +/// throughout the entire API. Every one of these traits requires that the +/// implementor also implements AptosErrorResponse, which saves functions from +/// having to add that bound to errors themselves. +#[macro_export] +macro_rules! generate_error_traits { + ($($trait_name:ident),*) => { + paste::paste! { + $( + pub trait [<$trait_name Error>]: AptosErrorResponse { + fn [<$trait_name:snake>](error: anyhow::Error) -> Self where Self: Sized; + fn [<$trait_name:snake _str>](error_str: &str) -> Self where Self: Sized; + } + )* + } + }; } -impl AptosResponse { - fn from_ledger_info(content: AptosResponseContent, ledger_info: &LedgerInfo) -> Self { - AptosResponse::Ok( - content, - ledger_info.chain_id as u16, - ledger_info.ledger_version.into(), - ledger_info.oldest_ledger_version.into(), - ledger_info.ledger_timestamp.into(), - ledger_info.epoch, - ) - } +/// This macro helps generate types, From impls, etc. for an error +/// response from a Poem endpoint. It generates a response type that only has +/// the specified response codes, which is then reflected in the OpenAPI spec. +/// For each status code given to a particular invocation of this macro, we +/// implement the relevant trait from generate_error_traits. +/// See the comments in the macro for an explanation of what is happening. +#[macro_export] +macro_rules! generate_error_response { + ($enum_name:ident, $(($status:literal, $name:ident)),*) => { + // We use the paste crate to allows us to generate the name of the code + // enum, more on that in the comment above that enum. + paste::paste! { - /// Construct a response from bytes that you know ahead of time a BCS - /// encoded value. - pub fn from_bcs(value: Vec, ledger_info: &LedgerInfo) -> Self { - Self::from_ledger_info(AptosResponseContent::Bcs(Bcs(value)), ledger_info) - } + // Generate an enum with name `enum_name`. Iterate through each of the + // response codes, generating a variant for each with the given name + // and status code. We always generate a variant for 500. + #[derive(poem_openapi::ApiResponse)] + pub enum $enum_name { + $( + #[oai(status = $status)] + $name(poem_openapi::payload::Json<$crate::poem_backend::AptosError>), + )* + } - /// Construct an Aptos response from a Rust type, serializing it to JSON. - pub fn from_json(value: T, ledger_info: &LedgerInfo) -> Self { - Self::from_ledger_info(AptosResponseContent::Json(Json(value)), ledger_info) - } + // Generate an enum that captures all the different status codes that + // this response type supports. To explain this funky syntax, if you + // named the main enum MyResponse, this would become MyResponseCode. + pub enum [<$enum_name Status>] { + $( + $name, + )* + } - /// This is a convenience function for creating a response when you have - /// a Rust object from the beginning. If you're starting out with bytes, - /// you should instead check the accept type and use either `from_bcs` - /// or `from_json`. - pub fn try_from_rust_value( - value: T, - ledger_info: &LedgerInfo, - accept_type: &AcceptType, - ) -> Result { - match accept_type { - AcceptType::Bcs => Ok(AptosResponse::from_bcs( - serialize_to_bcs(&value)?, - ledger_info, - )), - AcceptType::Json => Ok(AptosResponse::from_json(value, ledger_info)), + // Generate a From impl that takes in an AptosError and a status code + // and returns an instance of $enum_name. + impl From<($crate::poem_backend::AptosError, [<$enum_name Status>])> + for $enum_name + { + fn from( + (error, code): ( + $crate::poem_backend::AptosError, + [<$enum_name Status>] + ) + ) -> Self { + match code { + $( + [<$enum_name Status>]::$name => $enum_name::$name(poem_openapi::payload::Json(error)), + )* + } + } + } + + // For each status, implement the relevant error trait. This means if + // the macro invocation specifies Internal and BadRequest, the + // functions internal(anyhow::Error) and bad_request(anyhow::Error) + // will be generated. There are also variants for taking in strs. + $( + impl $crate::poem_backend::[<$name Error>] for $enum_name { + fn [<$name:snake>](error: anyhow::Error) -> Self where Self: Sized { + Self::from(($crate::poem_backend::AptosError::from(error), [<$enum_name Status>]::$name)) + } + + fn [<$name:snake _str>](error_str: &str) -> Self where Self: Sized { + Self::from(($crate::poem_backend::AptosError::new(error_str.to_string()), [<$enum_name Status>]::$name)) + } + } + )* } - } -} -/// Serialize an internal Rust type to BCS, returning a 500 if it fails. -pub fn serialize_to_bcs(value: T) -> Result, AptosErrorResponse> { - bcs::to_bytes(&value).map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Rust type could not be serialized to BCS: {}", e).to_string(), - ) - .error_code(AptosErrorCode::BcsSerializationError), - )) - }) + // Generate a function that helps get the AptosError within. + impl $crate::poem_backend::AptosErrorResponse for $enum_name { + fn inner_mut(&mut self) -> &mut $crate::poem_backend::AptosError { + match self { + $( + $enum_name::$name(poem_openapi::payload::Json(inner)) => inner, + )* + } + } + } + }; } -/// Deserialize BCS bytes into an internal Rust, returning a 500 if it fails. -pub fn deserialize_from_bcs Deserialize<'b>>( - bytes: &[u8], -) -> Result { - bcs::from_bytes(bytes).map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new(format_err!("Data in storage was not valid BCS: {}", e).to_string()) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - }) +/// This macro helps generate types, From impls, etc. for a successful response +/// from a Poem endpoint. It generates a response type that only has the +/// specified response codes, which is then reflected in the OpenAPI spec. +/// See the comments in the macro for an explanation of what is happening. +#[macro_export] +macro_rules! generate_success_response { + ($enum_name:ident, $(($status:literal, $name:ident)),*) => { + // We use the paste crate to allows us to generate the name of the code + // enum, more on that in the comment above that enum. + paste::paste! { + + // Generate an enum with name `enum_name`. Iterate through each of the + // response codes, generating a variant for each with the given name + // and status code. + #[derive(poem_openapi::ApiResponse)] + pub enum $enum_name { + $( + #[oai(status = $status)] + $name( + $crate::poem_backend::AptosResponseContent, + #[oai(header = "X-Aptos-Chain-Id")] u16, + #[oai(header = "X-Aptos-Ledger-Version")] u64, + #[oai(header = "X-Aptos-Ledger-Oldest-Version")] u64, + #[oai(header = "X-Aptos-Ledger-TimestampUsec")] u64, + #[oai(header = "X-Aptos-Epoch")] u64, + ), + )* + } + + // Generate an enum that captures all the different status codes that + // this response type supports. To explain this funky syntax, if you + // named the main enum MyResponse, this would become MyResponseCode. + pub enum [<$enum_name Status>] { + $( + $name, + )* + } + + // Generate a From impl that builds a response from AptosResponseContent. + // Each variant in the main enum takes in the same argument, so the macro + // is really just helping us enumerate and build each variant. We use this + // in the other From impls. + impl From<($crate::poem_backend::AptosResponseContent, &aptos_api_types::LedgerInfo, [<$enum_name Status>])> + for $enum_name + { + fn from( + (value, ledger_info, status): ( + $crate::poem_backend::AptosResponseContent, + &aptos_api_types::LedgerInfo, + [<$enum_name Status>] + ), + ) -> Self { + match status { + $( + [<$enum_name Status>]::$name => { + $enum_name::$name( + value, + ledger_info.chain_id as u16, + ledger_info.ledger_version.into(), + ledger_info.oldest_ledger_version.into(), + ledger_info.ledger_timestamp.into(), + ledger_info.epoch, + ) + }, + )* + } + } + } + + // Generate a From impl that builds a response from a Json and friends. + impl From<(poem_openapi::payload::Json, &aptos_api_types::LedgerInfo, [<$enum_name Status>])> + for $enum_name + { + fn from( + (value, ledger_info, status): (poem_openapi::payload::Json, &aptos_api_types::LedgerInfo, [<$enum_name Status>]), + ) -> Self { + let content = $crate::poem_backend::AptosResponseContent::Json(value); + Self::from((content, ledger_info, status)) + } + } + + // Generate a From impl that builds a response from a Bcs> and friends. + impl From<($crate::poem_backend::bcs_payload::Bcs>, &aptos_api_types::LedgerInfo, [<$enum_name Status>])> + for $enum_name + { + fn from( + (value, ledger_info, status): ( + $crate::poem_backend::bcs_payload::Bcs>, + &aptos_api_types::LedgerInfo, + [<$enum_name Status>] + ), + ) -> Self { + let content = $crate::poem_backend::AptosResponseContent::Bcs(value); + Self::from((content, ledger_info, status)) + } + } + + // Generate a TryFrom impl that builds a response from a T, an AcceptType, + // and all the other usual suspects. It expects to be called with a generic + // parameter E: InternalError, with which we can build an internal error + // response in case the BCS serialization fails. + impl $enum_name { + pub fn try_from_rust_value( + (value, ledger_info, status, accept_type): ( + T, + &aptos_api_types::LedgerInfo, + [<$enum_name Status>], + &$crate::poem_backend::AcceptType + ), + ) -> Result { + match accept_type { + AcceptType::Bcs => Ok(Self::from(( + $crate::poem_backend::bcs_payload::Bcs( + bcs::to_bytes(&value) + .map_err(|e| E::internal(e.into()).error_code($crate::poem_backend::AptosErrorCode::BcsSerializationError))? + ), + ledger_info, + status + ))), + AcceptType::Json => Ok(Self::from(( + poem_openapi::payload::Json(value), + ledger_info, + status + ))), + } + } + } + } + }; } + +// Generate a success response that only has an option for 200. +generate_success_response!(BasicResponse, (200, Ok)); + +// Generate traits defining a "from" function for each of these status types. +// The error response then impls these traits for each status type they mention. +generate_error_traits!(Internal, BadRequest, NotFound); + +// Generate an error response that only has options for 400 and 500. +generate_error_response!(BasicError, (400, BadRequest), (500, Internal)); + +// This type just simplifies using BasicResponse and BasicError together. +pub type BasicResult = poem::Result, BasicError>; + +// As above but with 404. +generate_error_response!( + BasicErrorWith404, + (400, BadRequest), + (404, NotFound), + (500, Internal) +); +pub type BasicResultWith404 = poem::Result, BasicErrorWith404>; diff --git a/api/src/poem_backend/transactions.rs b/api/src/poem_backend/transactions.rs index 303145991c145..2dee7b3c1abac 100644 --- a/api/src/poem_backend/transactions.rs +++ b/api/src/poem_backend/transactions.rs @@ -4,26 +4,23 @@ // Copyright (c) Aptos // SPDX-License-Identifier: Apache-2.0 -use std::convert::TryFrom; use std::sync::Arc; -use super::accept_type::AcceptType; +use super::accept_type::{parse_accept, AcceptType}; use super::page::Page; -use super::{response::AptosResponseResult, ApiTags, AptosResponse}; -use super::{AptosError, AptosErrorCode, AptosErrorResponse}; +use super::AptosErrorCode; +use super::{ + ApiTags, AptosErrorResponse, BasicErrorWith404, BasicResponse, BasicResponseStatus, + BasicResultWith404, InternalError, +}; use crate::context::Context; use crate::failpoint::fail_point_poem; -use anyhow::format_err; -use aptos_api_types::AsConverter; -use aptos_api_types::{LedgerInfo, Transaction, TransactionOnChainData}; +use anyhow::Context as AnyhowContext; +use aptos_api_types::{AsConverter, LedgerInfo, Transaction, TransactionOnChainData}; use poem::web::Accept; use poem_openapi::param::Query; -use poem_openapi::payload::Json; use poem_openapi::OpenApi; -// TODO: Make a helper that builds an AptosResponse from just an anyhow error, -// that assumes that it's an internal error. We can use .context() add more info. - pub struct TransactionsApi { pub context: Arc, } @@ -44,38 +41,34 @@ impl TransactionsApi { accept: Accept, start: Query>, limit: Query>, - ) -> AptosResponseResult> { - fail_point_poem("endpoint_get_transactions")?; - let accept_type = AcceptType::try_from(&accept)?; + ) -> BasicResultWith404> { + fail_point_poem::("endppoint_get_transactions")?; + let accept_type = parse_accept::(&accept)?; let page = Page::new(start.0, limit.0); self.list(&accept_type, page) } } impl TransactionsApi { - fn list(&self, accept_type: &AcceptType, page: Page) -> AptosResponseResult> { - let latest_ledger_info = self.context.get_latest_ledger_info_poem()?; + fn list(&self, accept_type: &AcceptType, page: Page) -> BasicResultWith404> { + let latest_ledger_info = self + .context + .get_latest_ledger_info_poem::()?; let ledger_version = latest_ledger_info.version(); - let limit = page.limit()?; + let limit = page.limit::()?; let last_page_start = if ledger_version > (limit as u64) { ledger_version - (limit as u64) } else { 0 }; - let start_version = page.start(last_page_start, ledger_version)?; + let start_version = page.start::(last_page_start, ledger_version)?; let data = self .context .get_transactions(start_version, limit, ledger_version) - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to read raw transactions from storage: {}", e) - .to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - })?; + .context("Failed to read raw transactions from storage") + .map_err(BasicErrorWith404::internal) + .map_err(|e| e.error_code(AptosErrorCode::InvalidBcsInStorageError))?; self.render_transactions(data, accept_type, &latest_ledger_info) } @@ -85,12 +78,18 @@ impl TransactionsApi { data: Vec, accept_type: &AcceptType, latest_ledger_info: &LedgerInfo, - ) -> AptosResponseResult> { + ) -> BasicResultWith404> { if data.is_empty() { - return AptosResponse::try_from_rust_value(vec![], latest_ledger_info, accept_type); + let data: Vec = vec![]; + return BasicResponse::try_from_rust_value::(( + data, + latest_ledger_info, + BasicResponseStatus::Ok, + accept_type, + )); } - let resolver = self.context.move_resolver_poem()?; + let resolver = self.context.move_resolver_poem::()?; let converter = resolver.as_converter(); let txns: Vec = data .into_iter() @@ -101,16 +100,14 @@ impl TransactionsApi { Ok(txn) }) .collect::>() - .map_err(|e| { - AptosErrorResponse::InternalServerError(Json( - AptosError::new( - format_err!("Failed to convert transaction data from storage: {}", e) - .to_string(), - ) - .error_code(AptosErrorCode::InvalidBcsInStorageError), - )) - })?; + .context("Failed to convert transaction data from storage") + .map_err(BasicErrorWith404::internal)?; - AptosResponse::try_from_rust_value(txns, latest_ledger_info, accept_type) + BasicResponse::try_from_rust_value(( + txns, + latest_ledger_info, + BasicResponseStatus::Ok, + accept_type, + )) } } diff --git a/api/types/src/address.rs b/api/types/src/address.rs index 6146c531e9d5a..7f717e6c36986 100644 --- a/api/types/src/address.rs +++ b/api/types/src/address.rs @@ -76,12 +76,6 @@ impl<'de> Deserialize<'de> for Address { } } -impl Default for Address { - fn default() -> Self { - Self(AccountAddress::ONE) - } -} - impl_poem_type!(Address); impl_poem_parameter!(Address);