Error handling using failure in API (#949)

This PR adresses #166
Error handling in wallet was ported to failure in https://github.com/mimblewimble/grin/pull/713
Using the same error model makes wallet code simpler and may simplify migration to Hyper.

Cargo.lock

@@ -630,6 +630,8 @@ dependencies = [
 name = "grin_api"
 version = "0.2.0"
 dependencies = [
+ "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "failure_derive 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "grin_chain 0.2.0",
  "grin_core 0.2.0",
  "grin_p2p 0.2.0",

api/Cargo.toml

@@ -6,6 +6,8 @@ workspace = ".."
 publish = false
 
 [dependencies]
+failure = "0.1"
+failure_derive = "0.1"
 hyper = "0.10"
 iron = "0.5"
 lazy_static = "0.2"

api/src/client.rs

@@ -14,6 +14,7 @@
 
 //! High level JSON/HTTP client API
 
+use failure::{Fail, ResultExt};
 use hyper;
 use hyper::client::Response;
 use hyper::status::{StatusClass, StatusCode};
@@ -21,7 +22,7 @@ use serde::{Deserialize, Serialize};
 use serde_json;
 use std::io::Read;
 
-use rest::Error;
+use rest::{Error, ErrorKind};
 
 /// Helper function to easily issue a HTTP GET request against a given URL that
 /// returns a JSON object. Handles request building, JSON deserialization and
@@ -32,8 +33,11 @@ where
 {
 	let client = hyper::Client::new();
 	let res = check_error(client.get(url).send())?;
-	serde_json::from_reader(res)
-		.map_err(|e| Error::Internal(format!("Server returned invalid JSON: {}", e)))
+	serde_json::from_reader(res).map_err(|e| {
+		e.context(ErrorKind::Internal(
+			"Server returned invalid JSON".to_owned(),
+		)).into()
+	})
 }
 
 /// Helper function to easily issue a HTTP POST request with the provided JSON
@@ -44,8 +48,9 @@ pub fn post<'a, IN>(url: &'a str, input: &IN) -> Result<(), Error>
 where
 	IN: Serialize,
 {
-	let in_json = serde_json::to_string(input)
-		.map_err(|e| Error::Internal(format!("Could not serialize data to JSON: {}", e)))?;
+	let in_json = serde_json::to_string(input).context(ErrorKind::Internal(
+		"Could not serialize data to JSON".to_owned(),
+	))?;
 	let client = hyper::Client::new();
 	let _res = check_error(client.post(url).body(&mut in_json.as_bytes()).send())?;
 	Ok(())
@@ -54,24 +59,27 @@ where
 // convert hyper error and check for non success response codes
 fn check_error(res: hyper::Result<Response>) -> Result<Response, Error> {
 	if let Err(e) = res {
-		return Err(Error::Internal(format!("Error during request: {}", e)));
+		return Err(
+			e.context(ErrorKind::Internal("Error during request".to_owned()))
+				.into(),
+		);
 	}
 	let mut response = res.unwrap();
 	match response.status.class() {
 		StatusClass::Success => Ok(response),
-		StatusClass::ServerError => Err(Error::Internal(format!(
+		StatusClass::ServerError => Err(ErrorKind::Internal(format!(
 			"Server error: {}",
 			err_msg(&mut response)
-		))),
+		)))?,
 		StatusClass::ClientError => if response.status == StatusCode::NotFound {
-			Err(Error::NotFound)
+			Err(ErrorKind::NotFound)?
 		} else {
-			Err(Error::Argument(format!(
+			Err(ErrorKind::Argument(format!(
 				"Argument error: {}",
 				err_msg(&mut response)
-			)))
+			)))?
 		},
-		_ => Err(Error::Internal(format!("Unrecognized error."))),
+		_ => Err(ErrorKind::Internal(format!("Unrecognized error.")))?,
 	}
 }
 

api/src/handlers.rs

@@ -16,25 +16,26 @@ use std::io::Read;
 use std::sync::{Arc, RwLock, Weak};
 use std::thread;
 
-use iron::prelude::*;
+use failure::{Fail, ResultExt};
 use iron::Handler;
+use iron::prelude::*;
 use iron::status;
-use urlencoded::UrlEncodedQuery;
 use serde::Serialize;
 use serde_json;
+use urlencoded::UrlEncodedQuery;
 
 use chain;
-use core::core::{OutputFeatures, OutputIdentifier, Transaction};
 use core::core::hash::{Hash, Hashed};
+use core::core::{OutputFeatures, OutputIdentifier, Transaction};
 use core::ser;
-use pool;
 use p2p;
+use pool;
 use regex::Regex;
 use rest::*;
-use util::secp::pedersen::Commitment;
 use types::*;
 use util;
 use util::LOGGER;
+use util::secp::pedersen::Commitment;
 
 // All handlers use `Weak` references instead of `Arc` to avoid cycles that
 // can never be destroyed. These 2 functions are simple helpers to reduce the
@@ -67,8 +68,10 @@ struct OutputHandler {
 
 impl OutputHandler {
 	fn get_output(&self, id: &str) -> Result<Output, Error> {
-		let c = util::from_hex(String::from(id))
-			.map_err(|_| Error::Argument(format!("Not a valid commitment: {}", id)))?;
+		let c = util::from_hex(String::from(id)).context(ErrorKind::Argument(format!(
+			"Not a valid commitment: {}",
+			id
+		)))?;
 		let commit = Commitment::from_vec(c);
 
 		// We need the features here to be able to generate the necessary hash
@@ -85,7 +88,7 @@ impl OutputHandler {
 				return Ok(Output::new(&commit));
 			}
 		}
-		Err(Error::NotFound)
+		Err(ErrorKind::NotFound)?
 	}
 
 	fn outputs_by_ids(&self, req: &mut Request) -> Vec<Output> {
@@ -499,19 +502,18 @@ impl Handler for ChainCompactHandler {
 ///
 /// Optionally return results as "compact blocks" by passing "?compact" query param
 /// GET /v1/blocks/<hash>?compact
-///
 pub struct BlockHandler {
 	pub chain: Weak<chain::Chain>,
 }
 
 impl BlockHandler {
 	fn get_block(&self, h: &Hash) -> Result<BlockPrintable, Error> {
-		let block = w(&self.chain).get_block(h).map_err(|_| Error::NotFound)?;
+		let block = w(&self.chain).get_block(h).context(ErrorKind::NotFound)?;
 		Ok(BlockPrintable::from_block(&block, w(&self.chain), false))
 	}
 
 	fn get_compact_block(&self, h: &Hash) -> Result<CompactBlockPrintable, Error> {
-		let block = w(&self.chain).get_block(h).map_err(|_| Error::NotFound)?;
+		let block = w(&self.chain).get_block(h).context(ErrorKind::NotFound)?;
 		Ok(CompactBlockPrintable::from_compact_block(
 			&block.as_compact_block(),
 			w(&self.chain),
@@ -523,14 +525,16 @@ impl BlockHandler {
 		if let Ok(height) = input.parse() {
 			match w(&self.chain).get_header_by_height(height) {
 				Ok(header) => return Ok(header.hash()),
-				Err(_) => return Err(Error::NotFound),
+				Err(_) => return Err(ErrorKind::NotFound)?,
 			}
 		}
 		lazy_static! {
 			static ref RE: Regex = Regex::new(r"[0-9a-fA-F]{64}").unwrap();
 		}
 		if !RE.is_match(&input) {
-			return Err(Error::Argument(String::from("Not a valid hash or height.")));
+			return Err(ErrorKind::Argument(
+				"Not a valid hash or height.".to_owned(),
+			))?;
 		}
 		let vec = util::from_hex(input).unwrap();
 		Ok(Hash::from_vec(vec))
@@ -545,7 +549,8 @@ impl Handler for BlockHandler {
 			path_elems.pop();
 		}
 		let el = *path_elems.last().unwrap();
-		let h = try!(self.parse_input(el.to_string()));
+		let h = self.parse_input(el.to_string())
+			.map_err(|e| IronError::new(Fail::compat(e), status::BadRequest))?;
 
 		let mut compact = false;
 		if let Ok(params) = req.get_ref::<UrlEncodedQuery>() {
@@ -555,10 +560,12 @@ impl Handler for BlockHandler {
 		}
 
 		if compact {
-			let b = try!(self.get_compact_block(&h));
+			let b = self.get_compact_block(&h)
+				.map_err(|e| IronError::new(Fail::compat(e), status::InternalServerError))?;
 			json_response(&b)
 		} else {
-			let b = try!(self.get_block(&h));
+			let b = self.get_block(&h)
+				.map_err(|e| IronError::new(Fail::compat(e), status::InternalServerError))?;
 			json_response(&b)
 		}
 	}
@@ -603,14 +610,13 @@ where
 {
 	fn handle(&self, req: &mut Request) -> IronResult<Response> {
 		let wrapper: TxWrapper = serde_json::from_reader(req.body.by_ref())
-			.map_err(|e| IronError::new(e, status::BadRequest))?;
+			.map_err(|e| IronError::new(Fail::compat(e), status::BadRequest))?;
 
 		let tx_bin = util::from_hex(wrapper.tx_hex)
-			.map_err(|_| Error::Argument(format!("Invalid hex in transaction wrapper.")))?;
+			.map_err(|e| IronError::new(Fail::compat(e), status::BadRequest))?;
 
-		let tx: Transaction = ser::deserialize(&mut &tx_bin[..]).map_err(|_| {
-			Error::Argument("Could not deserialize transaction, invalid format.".to_string())
-		})?;
+		let tx: Transaction = ser::deserialize(&mut &tx_bin[..])
+			.map_err(|e| IronError::new(Fail::compat(e), status::BadRequest))?;
 
 		let source = pool::TxSource {
 			debug_name: "push-api".to_string(),
@@ -650,7 +656,7 @@ where
 			Ok(()) => Ok(Response::with(status::Ok)),
 			Err(e) => {
 				debug!(LOGGER, "error - {:?}", e);
-				Err(IronError::from(Error::Argument(format!("{:?}", e))))
+				Err(IronError::new(Fail::compat(e), status::BadRequest))
 			}
 		}
 	}

api/src/lib.rs

@@ -19,6 +19,10 @@ extern crate grin_pool as pool;
 extern crate grin_store as store;
 extern crate grin_util as util;
 
+
+extern crate failure;
+#[macro_use]
+extern crate failure_derive;
 extern crate hyper;
 extern crate iron;
 #[macro_use]

api/src/rest.rs

@@ -18,64 +18,70 @@
 //! To use it, just have your service(s) implement the ApiEndpoint trait and
 //! register them on a ApiServer.
 
-use std::error;
 use std::fmt::{self, Display, Formatter};
+use std::mem;
 use std::net::ToSocketAddrs;
 use std::string::ToString;
-use std::mem;
 
+use failure::{Backtrace, Context, Fail, ResultExt};
+use iron::middleware::Handler;
 use iron::prelude::*;
 use iron::{status, Listening};
-use iron::middleware::Handler;
-use router::Router;
 use mount::Mount;
+use router::Router;
 
 use store;
 
 /// Errors that can be returned by an ApiEndpoint implementation.
+
 #[derive(Debug)]
-pub enum Error {
+pub struct Error {
+	inner: Context<ErrorKind>,
+}
+
+#[derive(Clone, Eq, PartialEq, Debug, Fail)]
+pub enum ErrorKind {
+	#[fail(display = "Internal error: {}", _0)]
 	Internal(String),
+	#[fail(display = "Bad arguments: {}", _0)]
 	Argument(String),
+	#[fail(display = "Not found.")]
 	NotFound,
 }
 
+impl Fail for Error {
+	fn cause(&self) -> Option<&Fail> {
+		self.inner.cause()
+	}
+
+	fn backtrace(&self) -> Option<&Backtrace> {
+		self.inner.backtrace()
+	}
+}
+
 impl Display for Error {
-	fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-		match *self {
-			Error::Argument(ref s) => write!(f, "Bad arguments: {}", s),
-			Error::Internal(ref s) => write!(f, "Internal error: {}", s),
-			Error::NotFound => write!(f, "Not found."),
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		Display::fmt(&self.inner, f)
+	}
+}
+
+impl Error {
+	pub fn kind(&self) -> &ErrorKind {
+		self.inner.get_context()
+	}
+}
+
+impl From<ErrorKind> for Error {
+	fn from(kind: ErrorKind) -> Error {
+		Error {
+			inner: Context::new(kind),
 		}
 	}
 }
 
-impl error::Error for Error {
-	fn description(&self) -> &str {
-		match *self {
-			Error::Argument(_) => "Bad arguments.",
-			Error::Internal(_) => "Internal error.",
-			Error::NotFound => "Not found.",
-		}
-	}
-}
-
-impl From<Error> for IronError {
-	fn from(e: Error) -> IronError {
-		match e {
-			Error::Argument(_) => IronError::new(e, status::Status::BadRequest),
-			Error::Internal(_) => IronError::new(e, status::Status::InternalServerError),
-			Error::NotFound => IronError::new(e, status::Status::NotFound),
-		}
-	}
-}
-
-impl From<store::Error> for Error {
-	fn from(e: store::Error) -> Error {
-		match e {
-			store::Error::NotFoundErr => Error::NotFound,
-			_ => Error::Internal(e.to_string()),
-		}
+impl From<Context<ErrorKind>> for Error {
+	fn from(inner: Context<ErrorKind>) -> Error {
+		Error { inner: inner }
 	}
 }
 

chain/src/types.rs

@@ -14,19 +14,19 @@
 
 //! Base types that the block chain pipeline requires.
 
-use std::io;
+use std::{error, fmt, io};
 
 use util::secp;
 use util::secp::pedersen::Commitment;
 
-use grin_store as store;
-use core::core::{block, transaction, Block, BlockHeader};
 use core::core::hash::{Hash, Hashed};
 use core::core::target::Difficulty;
+use core::core::{block, transaction, Block, BlockHeader};
 use core::ser::{self, Readable, Reader, Writeable, Writer};
-use keychain;
+use grin_store as store;
 use grin_store;
 use grin_store::pmmr::PMMRFileMetadata;
+use keychain;
 
 bitflags! {
 /// Options for block validation
@@ -109,6 +109,22 @@ pub enum Error {
 	Other(String),
 }
 
+impl error::Error for Error {
+	fn description(&self) -> &str {
+		match *self {
+			_ => "some kind of chain error",
+		}
+	}
+}
+
+impl fmt::Display for Error {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		match *self {
+			_ => write!(f, "some kind of chain error"),
+		}
+	}
+}
+
 impl From<grin_store::Error> for Error {
 	fn from(e: grin_store::Error) -> Error {
 		Error::StoreErr(e, "wrapped".to_owned())
@@ -285,7 +301,8 @@ pub trait ChainStore: Send + Sync {
 	fn delete_header_by_height(&self, height: u64) -> Result<(), store::Error>;
 
 	/// Is the block header on the current chain?
-	/// Use the header_by_height index to verify the block header is where we think it is.
+	/// Use the header_by_height index to verify the block header is where we
+	/// think it is.
 	fn is_on_current_chain(&self, header: &BlockHeader) -> Result<(), store::Error>;
 
 	/// Saves the position of an output, represented by its commitment, in the
@@ -299,8 +316,8 @@ pub trait ChainStore: Send + Sync {
 	/// Deletes the MMR position of an output.
 	fn delete_output_pos(&self, commit: &[u8]) -> Result<(), store::Error>;
 
-	/// Saves a marker associated with a block recording the MMR positions of its
-	/// last elements.
+	/// Saves a marker associated with a block recording the MMR positions of
+	/// its last elements.
 	fn save_block_marker(&self, bh: &Hash, marker: &(u64, u64)) -> Result<(), store::Error>;
 
 	/// Retrieves a block marker from a block hash.

pool/src/types.rs

@@ -15,18 +15,18 @@
 //! The primary module containing the implementations of the transaction pool
 //! and its top-level members.
 
-use std::vec::Vec;
 use std::collections::{HashMap, HashSet};
 use std::iter::Iterator;
-use std::fmt;
+use std::vec::Vec;
+use std::{error, fmt};
 
 use util::secp::pedersen::Commitment;
 
 pub use graph;
 
 use core::consensus;
-use core::core::{block, hash, transaction};
 use core::core::transaction::{Input, OutputIdentifier};
+use core::core::{block, hash, transaction};
 
 /// Transaction pool configuration
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -162,14 +162,30 @@ pub enum PoolError {
 	LowFeeTransaction(u64),
 }
 
+impl error::Error for PoolError {
+	fn description(&self) -> &str {
+		match *self {
+			_ => "some kind of pool error",
+		}
+	}
+}
+
+impl fmt::Display for PoolError {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		match *self {
+			_ => write!(f, "some kind of pool error"),
+		}
+	}
+}
+
 /// Interface that the pool requires from a blockchain implementation.
 pub trait BlockChain {
-	/// Get an unspent output by its commitment. Will return an error if the output
-	/// is spent or if it doesn't exist. The blockchain is expected to produce
-	/// a result with its current view of the most worked chain, ignoring
-	/// orphans, etc.
-	/// We do not maintain outputs themselves. The only information we have is the
-	/// hash from the output MMR.
+	/// Get an unspent output by its commitment. Will return an error if the
+	/// output is spent or if it doesn't exist. The blockchain is expected to
+	/// produce a result with its current view of the most worked chain,
+	/// ignoring orphans, etc.
+	/// We do not maintain outputs themselves. The only information we have is
+	/// the hash from the output MMR.
 	fn is_unspent(&self, output_ref: &OutputIdentifier) -> Result<hash::Hash, PoolError>;
 
 	/// Check if an output being spent by the input has sufficiently matured.
@@ -188,8 +204,8 @@ pub trait PoolAdapter: Send + Sync {
 	/// The transaction pool has accepted this transactions as valid and added
 	/// it to its internal cache.
 	fn tx_accepted(&self, tx: &transaction::Transaction);
-	/// The stem transaction pool has accepted this transactions as valid and added
-	/// it to its internal cache.
+	/// The stem transaction pool has accepted this transactions as valid and
+	/// added it to its internal cache.
 	fn stem_tx_accepted(&self, tx: &transaction::Transaction);
 }
 

wallet/src/handlers.rs

@@ -12,19 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use iron::prelude::*;
+use bodyparser;
 use iron::Handler;
+use iron::prelude::*;
 use iron::status;
 use serde_json;
-use bodyparser;
 
-use receiver::receive_coinbase;
-use core::ser;
 use api;
+use core::ser;
+use failure::{Fail, ResultExt};
 use keychain::Keychain;
+use receiver::receive_coinbase;
 use types::*;
 use util;
-use failure::{Fail, ResultExt};
 
 pub struct CoinbaseHandler {
 	pub config: WalletConfig,
@@ -33,22 +33,15 @@ pub struct CoinbaseHandler {
 
 impl CoinbaseHandler {
 	fn build_coinbase(&self, block_fees: &BlockFees) -> Result<CbData, Error> {
-		let (out, kern, block_fees) = receive_coinbase(&self.config, &self.keychain, block_fees)
-			.map_err(|e| api::Error::Internal(format!("Error building coinbase: {:?}", e)))
-			.context(ErrorKind::Node)?;
+		let (out, kern, block_fees) =
+			receive_coinbase(&self.config, &self.keychain, block_fees).context(ErrorKind::Node)?;
 
-		let out_bin = ser::ser_vec(&out)
-			.map_err(|e| api::Error::Internal(format!("Error serializing output: {:?}", e)))
-			.context(ErrorKind::Node)?;
+		let out_bin = ser::ser_vec(&out).context(ErrorKind::Node)?;
 
-		let kern_bin = ser::ser_vec(&kern)
-			.map_err(|e| api::Error::Internal(format!("Error serializing kernel: {:?}", e)))
-			.context(ErrorKind::Node)?;
+		let kern_bin = ser::ser_vec(&kern).context(ErrorKind::Node)?;
 
 		let key_id_bin = match block_fees.key_id {
-			Some(key_id) => ser::ser_vec(&key_id)
-				.map_err(|e| api::Error::Internal(format!("Error serializing kernel: {:?}", e)))
-				.context(ErrorKind::Node)?,
+			Some(key_id) => ser::ser_vec(&key_id).context(ErrorKind::Node)?,
 			None => vec![],
 		};
 

wallet/src/receiver.rs

@@ -17,8 +17,8 @@
 //! wallet server that's running at all time is required in many cases.
 
 use bodyparser;
-use iron::prelude::*;
 use iron::Handler;
+use iron::prelude::*;
 use iron::status;
 use serde_json;
 use uuid::Uuid;
@@ -27,11 +27,11 @@ use api;
 use core::consensus::reward;
 use core::core::{amount_to_hr_string, build, Block, Committed, Output, Transaction, TxKernel};
 use core::{global, ser};
+use failure::{Fail, ResultExt};
 use keychain::{BlindingFactor, Identifier, Keychain};
 use types::*;
-use util::{secp, to_hex, LOGGER};
 use urlencoded::UrlEncodedQuery;
-use failure::ResultExt;
+use util::{secp, to_hex, LOGGER};
 
 /// Dummy wrapper for the hex-encoded serialized transaction.
 #[derive(Serialize, Deserialize)]
@@ -42,8 +42,9 @@ pub struct TxWrapper {
 /// Receive Part 1 of interactive transactions from sender, Sender Initiation
 /// Return result of part 2, Recipient Initation, to sender
 /// -Receiver receives inputs, outputs xS * G and kS * G
-/// -Receiver picks random blinding factors for all outputs being received, computes total blinding
-///     excess xR
+/// -Receiver picks random blinding factors for all outputs being received,
+/// computes total blinding     
+/// excess xR
 /// -Receiver picks random nonce kR
 /// -Receiver computes Schnorr challenge e = H(M | kR * G + kS * G)
 /// -Receiver computes their part of signature, sR = kR + e * xR
@@ -146,7 +147,8 @@ fn handle_sender_initiation(
 /// Receive Part 3 of interactive transactions from sender, Sender Confirmation
 /// Return Ok/Error
 /// -Receiver receives sS
-/// -Receiver verifies sender's sig, by verifying that kS * G + e *xS * G = sS * G
+/// -Receiver verifies sender's sig, by verifying that 
+/// kS * G + e *xS * G = sS* G 
 /// -Receiver calculates final sig as s=(sS+sR, kS * G+kR * G)
 /// -Receiver puts into TX kernel:
 ///
@@ -281,9 +283,8 @@ impl Handler for WalletReceiver {
 						&partial_tx,
 					).map_err(|e| {
 						error!(LOGGER, "Phase 1 Sender Initiation -> Problematic partial tx, looks like this: {:?}", partial_tx);
-						api::Error::Internal(format!(
-							"Error processing partial transaction: {:?}",
-							e
+						e.context(api::ErrorKind::Internal(
+							"Error processing partial transaction".to_owned(),
 						))
 					})
 						.unwrap();
@@ -298,9 +299,8 @@ impl Handler for WalletReceiver {
 						fluff,
 					).map_err(|e| {
 						error!(LOGGER, "Phase 3 Sender Confirmation -> Problematic partial tx, looks like this: {:?}", partial_tx);
-						api::Error::Internal(format!(
-							"Error processing partial transaction: {:?}",
-							e
+						e.context(api::ErrorKind::Internal(
+							"Error processing partial transaction".to_owned(),
 						))
 					})
 						.unwrap();