Show HTTP status when node requests fail (#820)

(cherry picked from commit ec1e9910af)

impls/src/client_utils/client.rs

@@ -225,7 +225,7 @@ impl Client {
 		IN: Serialize,
 	{
 		let json = serde_json::to_string(input)
-			.map_err(|_| Error::Internal("Could not serialize data to JSON".to_owned()))?;
+			.map_err(|e| Error::Internal(format!("Could not serialize data to JSON: {}", e)))?;
 		self.build_request(url, Method::POST, api_secret, Some(json))
 	}
 
@@ -235,7 +235,7 @@ impl Client {
 	{
 		let data = self.send_request(req)?;
 		serde_json::from_str(&data)
-			.map_err(|_| Error::ResponseError("Cannot parse response".to_owned()))
+			.map_err(|e| Error::ResponseError(format!("Cannot parse response: {}", e)))
 	}
 
 	async fn handle_request_async<T>(&self, req: RequestBuilder) -> Result<T, Error>
@@ -244,7 +244,7 @@ impl Client {
 	{
 		let data = self.send_request_async(req).await?;
 		let ser = serde_json::from_str(&data)
-			.map_err(|_| Error::ResponseError("Cannot parse response".to_owned()))?;
+			.map_err(|e| Error::ResponseError(format!("Cannot parse response: {}", e)))?;
 		Ok(ser)
 	}
 
@@ -253,6 +253,10 @@ impl Client {
 			.send()
 			.await
 			.map_err(|e| Error::RequestError(format!("Cannot make request: {}", e)))?;
+		let status = resp.status();
+		if status.is_client_error() || status.is_server_error() {
+			return Err(Error::ResponseError(format!("HTTP {}", status)));
+		}
 		let text = resp
 			.text()
 			.await
@@ -261,7 +265,7 @@ impl Client {
 	}
 
 	pub fn send_request(&self, req: RequestBuilder) -> Result<String, Error> {
-		// This client is currently used both outside and inside of a tokio runtime
+		// This client is currently used both outside and inside the tokio runtime
 		// context. In the latter case we are not allowed to do a blocking call to
 		// our global runtime, which unfortunately means we have to spawn a new thread
 		if Handle::try_current().is_ok() {
@@ -275,3 +279,53 @@ impl Client {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
+	use tokio::net::TcpListener;
+
+	#[tokio::test]
+	async fn http_status() {
+		let client = Client {
+			client: ClientBuilder::new()
+				.no_proxy()
+				.timeout(Duration::from_secs(5))
+				.build()
+				.unwrap(),
+		};
+		for (status, body, expected) in [
+			("200 OK", "{}", Ok(serde_json::json!({}))),
+			(
+				"401 Unauthorized",
+				"<html>Unauthorized</html>",
+				Err(Error::ResponseError("HTTP 401 Unauthorized".into())),
+			),
+		] {
+			let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+			let url = format!("http://{}", listener.local_addr().unwrap());
+			let server = tokio::spawn(async move {
+				let (mut stream, _) = listener.accept().await.unwrap();
+				let mut headers = BufReader::new(&mut stream).lines();
+				while let Some(line) = headers.next_line().await.unwrap() {
+					if line.is_empty() {
+						break;
+					}
+				}
+				let response = format!(
+					"HTTP/1.1 {}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}",
+					status,
+					body.len(),
+					body
+				);
+				stream.write_all(response.as_bytes()).await.unwrap();
+			});
+			assert_eq!(
+				client._get_async::<serde_json::Value>(&url, None).await,
+				expected
+			);
+			server.await.unwrap();
+		}
+	}
+}

impls/src/node_clients/http.rs

@@ -142,19 +142,12 @@ impl NodeClient for HTTPNodeClient {
 			},
 			Err(e) => {
 				// If node isn't available, allow offline functions
-				// unfortunately have to parse string due to error structure
-				let err_string = format!("{}", e);
-				return if err_string.contains("404") {
-					Some(NodeVersionInfo {
-						node_version: "1.0.0".into(),
-						block_header_version: 1,
-						verified: Some(false),
-					})
-				} else {
-					error!("Unable to contact Node to get version info: {}, check your node is running", e);
-					warn!("Warning: a) Node is offline, or b) 'node_api_secret_path' in 'grin-wallet.toml' is set incorrectly");
-					None
-				};
+				error!(
+					"Unable to contact Node to get version info: {}, check your node is running",
+					e
+				);
+				warn!("Warning: a) Node is offline, or b) 'node_api_secret_path' in 'grin-wallet.toml' is set incorrectly");
+				return None;
 			}
 		};
 		self.node_version_info = Some(retval.clone());