From a97ab376cbd3f1e0ee414c9155e2f780b613d2d5 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Fri, 25 Jan 2019 12:12:50 +0000 Subject: [PATCH] Improve wallet + node API Comms error logging (#2472) * make wallet+node communication errors more verbose * rustfmt --- wallet/src/adapters/http.rs | 13 +++++++--- wallet/src/adapters/keybase.rs | 12 +++++++-- wallet/src/controller.rs | 4 ++- wallet/src/libwallet/error.rs | 2 +- wallet/src/node_clients/http.rs | 32 +++++++++++++++--------- wallet/src/test_framework/testclient.rs | 33 ++++++++++++++----------- 6 files changed, 61 insertions(+), 35 deletions(-) diff --git a/wallet/src/adapters/http.rs b/wallet/src/adapters/http.rs index 941cc0cd8..03e36826a 100644 --- a/wallet/src/adapters/http.rs +++ b/wallet/src/adapters/http.rs @@ -48,10 +48,15 @@ impl WalletCommAdapter for HTTPWalletCommAdapter { let url = format!("{}/v1/wallet/foreign/receive_tx", dest); debug!("Posting transaction slate to {}", url); - let res = api::client::post(url.as_str(), None, slate).context( - ErrorKind::ClientCallback("Posting transaction slate (is recipient listening?)"), - )?; - Ok(res) + let res = api::client::post(url.as_str(), None, slate); + match res { + Err(e) => { + let report = format!("Posting transaction slate (is recipient listening?): {}", e); + error!("{}", report); + Err(ErrorKind::ClientCallback(report).into()) + } + Ok(r) => Ok(r), + } } fn send_tx_async(&self, _dest: &str, _slate: &Slate) -> Result<(), Error> { diff --git a/wallet/src/adapters/keybase.rs b/wallet/src/adapters/keybase.rs index 75565af95..2e411919d 100644 --- a/wallet/src/adapters/keybase.rs +++ b/wallet/src/adapters/keybase.rs @@ -291,7 +291,11 @@ impl WalletCommAdapter for KeybaseWalletCommAdapter { // Send original slate to recipient with the SLATE_NEW topic match send(slate, addr, SLATE_NEW, TTL) { true => (), - false => return Err(ErrorKind::ClientCallback("Posting transaction slate"))?, + false => { + return Err(ErrorKind::ClientCallback( + "Posting transaction slate".to_owned(), + ))? + } } info!( "tx request has been sent to @{}, tx uuid: {}", @@ -300,7 +304,11 @@ impl WalletCommAdapter for KeybaseWalletCommAdapter { // Wait for response from recipient with SLATE_SIGNED topic match poll(TTL as u64, addr) { Some(slate) => return Ok(slate), - None => return Err(ErrorKind::ClientCallback("Receiving reply from recipient"))?, + None => { + return Err(ErrorKind::ClientCallback( + "Receiving reply from recipient".to_owned(), + ))? + } } } diff --git a/wallet/src/controller.rs b/wallet/src/controller.rs index 99e358370..cad0a9bdd 100644 --- a/wallet/src/controller.rs +++ b/wallet/src/controller.rs @@ -365,7 +365,9 @@ where } _ => { error!("unsupported payment method: {}", args.method); - return Err(ErrorKind::ClientCallback("unsupported payment method"))?; + return Err(ErrorKind::ClientCallback( + "unsupported payment method".to_owned(), + ))?; } } api.tx_lock_outputs(&slate, lock_fn)?; diff --git a/wallet/src/libwallet/error.rs b/wallet/src/libwallet/error.rs index 9016d763b..be108cddb 100644 --- a/wallet/src/libwallet/error.rs +++ b/wallet/src/libwallet/error.rs @@ -93,7 +93,7 @@ pub enum ErrorKind { /// API Error #[fail(display = "Client Callback Error: {}", _0)] - ClientCallback(&'static str), + ClientCallback(String), /// Secp Error #[fail(display = "Secp error")] diff --git a/wallet/src/node_clients/http.rs b/wallet/src/node_clients/http.rs index 3581ceab9..e07a9c6ee 100644 --- a/wallet/src/node_clients/http.rs +++ b/wallet/src/node_clients/http.rs @@ -69,9 +69,12 @@ impl NodeClient for HTTPNodeClient { } else { url = format!("{}/v1/pool/push", dest); } - api::client::post_no_ret(url.as_str(), self.node_api_secret(), tx).context( - libwallet::ErrorKind::ClientCallback("Posting transaction to node"), - )?; + let res = api::client::post_no_ret(url.as_str(), self.node_api_secret(), tx); + if let Err(e) = res { + let report = format!("Posting transaction to node: {}", e); + error!("Post TX Error: {}", e); + return Err(libwallet::ErrorKind::ClientCallback(report).into()); + } Ok(()) } @@ -79,10 +82,15 @@ impl NodeClient for HTTPNodeClient { fn get_chain_height(&self) -> Result { let addr = self.node_url(); let url = format!("{}/v1/chain", addr); - let res = api::client::get::(url.as_str(), self.node_api_secret()).context( - libwallet::ErrorKind::ClientCallback("Getting chain height from node"), - )?; - Ok(res.height) + let res = api::client::get::(url.as_str(), self.node_api_secret()); + match res { + Err(e) => { + let report = format!("Getting chain height from node: {}", e); + error!("Get chain height error: {}", e); + Err(libwallet::ErrorKind::ClientCallback(report).into()) + } + Ok(r) => Ok(r.height), + } } /// Retrieve outputs from node @@ -116,8 +124,9 @@ impl NodeClient for HTTPNodeClient { let results = match rt.block_on(task) { Ok(outputs) => outputs, Err(e) => { + let report = format!("Getting outputs by id: {}", e); error!("Outputs by id failed: {}", e); - return Err(libwallet::ErrorKind::ClientCallback("Error from server"))?; + return Err(libwallet::ErrorKind::ClientCallback(report).into()); } }; @@ -173,12 +182,11 @@ impl NodeClient for HTTPNodeClient { Err(e) => { // if we got anything other than 200 back from server, bye error!( - "get_outputs_by_pmmr_index: unable to contact API {}. Error: {}", + "get_outputs_by_pmmr_index: error contacting {}. Error: {}", addr, e ); - Err(libwallet::ErrorKind::ClientCallback( - "unable to contact api", - ))? + let report = format!("outputs by pmmr index: {}", e); + Err(libwallet::ErrorKind::ClientCallback(report))? } } } diff --git a/wallet/src/test_framework/testclient.rs b/wallet/src/test_framework/testclient.rs index 138953bcc..b6bb8fcac 100644 --- a/wallet/src/test_framework/testclient.rs +++ b/wallet/src/test_framework/testclient.rs @@ -177,15 +177,15 @@ where fn post_tx(&mut self, m: WalletProxyMessage) -> Result { let dest_wallet = self.wallets.get_mut(&m.sender_id).unwrap().1.clone(); let wrapper: TxWrapper = serde_json::from_str(&m.body).context( - libwallet::ErrorKind::ClientCallback("Error parsing TxWrapper"), + libwallet::ErrorKind::ClientCallback("Error parsing TxWrapper".to_owned()), )?; let tx_bin = util::from_hex(wrapper.tx_hex).context( - libwallet::ErrorKind::ClientCallback("Error parsing TxWrapper: tx_bin"), + libwallet::ErrorKind::ClientCallback("Error parsing TxWrapper: tx_bin".to_owned()), )?; let tx: Transaction = ser::deserialize(&mut &tx_bin[..]).context( - libwallet::ErrorKind::ClientCallback("Error parsing TxWrapper: tx"), + libwallet::ErrorKind::ClientCallback("Error parsing TxWrapper: tx".to_owned()), )?; super::award_block_to_wallet(&self.chain, vec![&tx], dest_wallet)?; @@ -323,15 +323,16 @@ impl LocalWalletClient { }; { let p = self.proxy_tx.lock(); - p.send(m) - .context(libwallet::ErrorKind::ClientCallback("Send TX Slate"))?; + p.send(m).context(libwallet::ErrorKind::ClientCallback( + "Send TX Slate".to_owned(), + ))?; } let r = self.rx.lock(); let m = r.recv().unwrap(); trace!("Received send_tx_slate response: {:?}", m.clone()); Ok( serde_json::from_str(&m.body).context(libwallet::ErrorKind::ClientCallback( - "Parsing send_tx_slate response", + "Parsing send_tx_slate response".to_owned(), ))?, ) } @@ -352,15 +353,16 @@ impl WalletCommAdapter for LocalWalletClient { }; { let p = self.proxy_tx.lock(); - p.send(m) - .context(libwallet::ErrorKind::ClientCallback("Send TX Slate"))?; + p.send(m).context(libwallet::ErrorKind::ClientCallback( + "Send TX Slate".to_owned(), + ))?; } let r = self.rx.lock(); let m = r.recv().unwrap(); trace!("Received send_tx_slate response: {:?}", m.clone()); Ok( serde_json::from_str(&m.body).context(libwallet::ErrorKind::ClientCallback( - "Parsing send_tx_slate response", + "Parsing send_tx_slate response".to_owned(), ))?, ) } @@ -405,8 +407,9 @@ impl NodeClient for LocalWalletClient { }; { let p = self.proxy_tx.lock(); - p.send(m) - .context(libwallet::ErrorKind::ClientCallback("post_tx send"))?; + p.send(m).context(libwallet::ErrorKind::ClientCallback( + "post_tx send".to_owned(), + ))?; } let r = self.rx.lock(); let m = r.recv().unwrap(); @@ -425,7 +428,7 @@ impl NodeClient for LocalWalletClient { { let p = self.proxy_tx.lock(); p.send(m).context(libwallet::ErrorKind::ClientCallback( - "Get chain height send", + "Get chain height send".to_owned(), ))?; } let r = self.rx.lock(); @@ -434,7 +437,7 @@ impl NodeClient for LocalWalletClient { Ok(m.body .parse::() .context(libwallet::ErrorKind::ClientCallback( - "Parsing get_height response", + "Parsing get_height response".to_owned(), ))?) } @@ -457,7 +460,7 @@ impl NodeClient for LocalWalletClient { { let p = self.proxy_tx.lock(); p.send(m).context(libwallet::ErrorKind::ClientCallback( - "Get outputs from node send", + "Get outputs from node send".to_owned(), ))?; } let r = self.rx.lock(); @@ -496,7 +499,7 @@ impl NodeClient for LocalWalletClient { { let p = self.proxy_tx.lock(); p.send(m).context(libwallet::ErrorKind::ClientCallback( - "Get outputs from node by PMMR index send", + "Get outputs from node by PMMR index send".to_owned(), ))?; }