Fixing etherscan price parsing (#4202)

* Fixing etherscan price parsing

* Handling all errors

ethcore/src/miner/miner.rs

@@ -148,7 +148,8 @@ impl GasPriceCalibrator {
 		if Instant::now() >= self.next_calibration {
 			let usd_per_tx = self.options.usd_per_tx;
 			trace!(target: "miner", "Getting price info");
-			let price_info = PriceInfo::get(move |price: PriceInfo| {
+
+			PriceInfo::get(move |price: PriceInfo| {
 				trace!(target: "miner", "Price info arrived: {:?}", price);
 				let usd_per_eth = price.ethusd;
 				let wei_per_usd: f32 = 1.0e18 / usd_per_eth;
@@ -158,11 +159,7 @@ impl GasPriceCalibrator {
 				set_price(U256::from(wei_per_gas as u64));
 			});
 
-			if price_info.is_ok() {
-				self.next_calibration = Instant::now() + self.options.recalibration_period;
-			} else {
-				warn!(target: "miner", "Unable to update Ether price.");
-			}
+			self.next_calibration = Instant::now() + self.options.recalibration_period;
 		}
 	}
 }

ethcore/src/miner/price_info.rs

@@ -21,7 +21,7 @@ use std::time::Duration;
 use std::str::FromStr;
 use std::sync::mpsc;
 use hyper::client::{Handler, Request, Response, Client};
-use hyper::{Next, Encoder, Decoder};
+use hyper::{Url, Next, Encoder, Decoder};
 use hyper::net::HttpStream;
 
 #[derive(Debug)]
@@ -29,12 +29,12 @@ pub struct PriceInfo {
 	pub ethusd: f32,
 }
 
-pub struct SetPriceHandler<F: Fn(PriceInfo) + Sync + Send + 'static> {
+pub struct SetPriceHandler<F> {
 	set_price: F,
 	channel: mpsc::Sender<()>,
 }
 
-impl<F: Fn(PriceInfo) + Sync + Send + 'static> Drop for SetPriceHandler<F> {
+impl<F> Drop for SetPriceHandler<F> {
 	fn drop(&mut self) {
 		let _ = self.channel.send(());
 	}
@@ -47,37 +47,61 @@ impl<F: Fn(PriceInfo) + Sync + Send + 'static> Handler<HttpStream> for SetPriceH
 
 	fn on_response_readable(&mut self, r: &mut Decoder<HttpStream>) -> Next {
 		let mut body = String::new();
-		let _ = r.read_to_string(&mut body).ok()
-			.and_then(|_| Json::from_str(&body).ok())
-			.and_then(|json| json.find_path(&["result", "ethusd"])
-				.and_then(|obj| match *obj {
-					Json::String(ref s) => Some((self.set_price)(PriceInfo {
-						ethusd: FromStr::from_str(s)
-							.expect("Etherscan API will always return properly formatted price; qed")
-					})),
-					_ => None,
-				}));
+		let info = r.read_to_string(&mut body)
+			.map_err(|e| format!("Unable to read response: {:?}", e))
+			.and_then(|_| self.process_response(&body));
+
+		if let Err(e) = info {
+			warn!("Failed to auto-update latest ETH price: {:?}", e);
+		}
 		Next::end()
 	}
+}
 
+impl<F: Fn(PriceInfo) + Sync + Send + 'static> SetPriceHandler<F> {
+	fn process_response(&self, body: &str) -> Result<(), String> {
+		let json = Json::from_str(body).map_err(|e| format!("Invalid JSON returned: {:?}", e))?;
+		let obj = json.find_path(&["result", "ethusd"]).ok_or("USD price not found".to_owned())?;
+		let ethusd = match *obj {
+			Json::String(ref s) => FromStr::from_str(s).ok(),
+			_ => None,
+		}.ok_or("Unexpected price format.".to_owned())?;
+
+		(self.set_price)(PriceInfo {
+			ethusd: ethusd,
+		});
+		Ok(())
+	}
 }
 
 impl PriceInfo {
-	pub fn get<F: Fn(PriceInfo) + Sync + Send + 'static>(set_price: F) -> Result<(), ()> {
-		// TODO: Handle each error type properly
-		let client = Client::new().map_err(|_| ())?;
+	pub fn get<F: Fn(PriceInfo) + Sync + Send + 'static>(set_price: F) {
 		thread::spawn(move || {
-			let (tx, rx) = mpsc::channel();
 			let url = FromStr::from_str("http://api.etherscan.io/api?module=stats&action=ethprice")
 				.expect("string known to be a valid URL; qed");
-			let _ = client.request(
-				url,
-				SetPriceHandler {
-					set_price: set_price,
-					channel: tx,
-				}).ok().and_then(|_| rx.recv().ok());
-			client.close();
+
+			if let Err(e) = Self::request(url, set_price) {
+				warn!("Failed to auto-update latest ETH price: {:?}", e);
+			}
 		});
+	}
+
+	fn request<F: Fn(PriceInfo) + Send + Sync + 'static>(url: Url, set_price: F) -> Result<(), String> {
+		let (tx, rx) = mpsc::channel();
+		let client = Client::new().map_err(|e| format!("Unable to start client: {:?}", e))?;
+
+		client.request(
+			url,
+			SetPriceHandler {
+				set_price: set_price,
+				channel: tx,
+			},
+		).map_err(|_| "Request failed.".to_owned())?;
+
+		// Wait for exit
+		let _ = rx.recv().map_err(|e| format!("Request interrupted: {:?}", e))?;
+		client.close();
+
 		Ok(())
 	}
 }
@@ -93,7 +117,7 @@ fn should_get_price_info() {
 	let done = Arc::new((Mutex::new(PriceInfo { ethusd: 0f32 }), Condvar::new()));
 	let rdone = done.clone();
 
-	PriceInfo::get(move |price| { let mut p = rdone.0.lock(); *p = price; rdone.1.notify_one(); }).unwrap();
+	PriceInfo::get(move |price| { let mut p = rdone.0.lock(); *p = price; rdone.1.notify_one(); });
 	let mut p = done.0.lock();
 	let t = done.1.wait_for(&mut p, Duration::from_millis(10000));
 	assert!(!t.timed_out());