501 fix occassional horizonresponse decodeerror

clients/wallet/src/error.rs

@@ -8,8 +8,12 @@ use thiserror::Error;
 pub enum Error {
 	#[error("Invalid secret key")]
 	InvalidSecretKey,
-	#[error("Error fetching horizon data: {0}")]
-	HorizonResponseError(#[from] ReqwestError),
+	#[error("Error fetching horizon data: reqwest: {reqwest:?}, other: {other:?}")]
+	HorizonResponseError {
+		reqwest: Option<ReqwestError>,
+		status: Option<u16>,
+		other: Option<String>,
+	},
 	#[error("Could not build transaction: {0}")]
 	BuildTransactionError(String),
 	#[error("Transaction submission failed. Title: {title}, Status: {status}, Reason: {reason}, Envelope XDR: {envelope_xdr:?}")]
@@ -43,7 +47,22 @@ pub enum Error {
 impl Error {
 	pub fn is_recoverable(&self) -> bool {
 		match self {
-			Error::HorizonResponseError(e) if e.is_timeout() => true,
+			Error::HorizonResponseError { status, reqwest, .. } => {
+				if let Some(e) = reqwest {
+					if e.is_timeout() {
+						return true
+					}
+				}
+
+				if let Some(status) = status {
+					// forbidden error
+					if *status == 403 {
+						return true
+					}
+				}
+
+				false
+			},
 			Error::HorizonSubmissionError { status, .. } if *status == 504 => true,
 			Error::CacheError(e) => match e.kind {
 				CacheErrorKind::CreateDirectoryFailed |
@@ -61,13 +80,31 @@ impl Error {
 		let server_errors = 500u16..599;
 
 		match self {
-			Error::HorizonResponseError(e) =>
-				e.status().map(|code| server_errors.contains(&code.as_u16())).unwrap_or(false),
+			Error::HorizonResponseError { status, reqwest, .. } => {
+				if let Some(e) = reqwest {
+					return e
+						.status()
+						.map(|code| server_errors.contains(&code.as_u16()))
+						.unwrap_or(false)
+				}
+
+				if let Some(status) = status {
+					return server_errors.contains(status)
+				}
+
+				// by default, assume that it will be a client error.
+				false
+			},
 			Error::HorizonSubmissionError { status, .. } => server_errors.contains(status),
 			_ => false,
 		}
 	}
 
+	pub fn response_decode_error(status: StatusCode, response_in_bytes: &[u8]) -> Self {
+		let resp_as_str = std::str::from_utf8(response_in_bytes).map(|s| s.to_string()).ok();
+		Error::HorizonResponseError { reqwest: None, status: Some(status), other: resp_as_str }
+	}
+
 	pub fn cache_error(kind: CacheErrorKind) -> Self {
 		Self::CacheError(CacheError { kind, path: None, envelope: None, sequence_number: None })
 	}

clients/wallet/src/horizon/horizon.rs

@@ -53,7 +53,11 @@ pub fn horizon_url(is_public_network: bool, is_need_fallback: bool) -> &'static
 impl HorizonClient for reqwest::Client {
 	async fn get_from_url<R: DeserializeOwned>(&self, url: &str) -> Result<R, Error> {
 		tracing::debug!("accessing url: {url:?}");
-		let response = self.get(url).send().await.map_err(Error::HorizonResponseError)?;
+		let response = self.get(url).send().await.map_err(|e| Error::HorizonResponseError {
+			reqwest: Some(e),
+			status: None,
+			other: None,
+		})?;
 		interpret_response::<R>(response).await
 	}
 
@@ -151,9 +155,9 @@ impl HorizonClient for reqwest::Client {
 			let base_url = horizon_url(is_public_network, need_fallback);
 			let url = format!("{}/transactions", base_url);
 
-			let response = ready(
-				self.post(url).form(&params).send().await.map_err(Error::HorizonResponseError),
-			)
+			let response = ready(self.post(url).form(&params).send().await.map_err(|e| {
+				Error::HorizonResponseError { reqwest: Some(e), status: None, other: None }
+			}))
 			.and_then(|response| async move {
 				interpret_response::<TransactionResponse>(response).await
 			})

clients/wallet/src/horizon/responses.rs

@@ -46,14 +46,28 @@ macro_rules! debug_str_or_vec_u8 {
 pub(crate) async fn interpret_response<T: DeserializeOwned>(
 	response: reqwest::Response,
 ) -> Result<T, Error> {
-	if response.status().is_success() {
-		return response.json::<T>().await.map_err(Error::HorizonResponseError)
+	let status = response.status();
+	tracing::info!("interpret_response(): status: {status:?}");
+
+	let response_body = response.bytes().await.map_err(|e| {
+		tracing::warn!("interpret_response(): cannot convert response to bytes: {e:?}");
+		Error::HorizonResponseError { reqwest: Some(e), status: Some(status.as_u16()), other: None }
+	})?;
+
+	if status.is_success() {
+		return serde_json::from_slice(&response_body).map_err(|e| {
+			tracing::warn!(
+				"interpret_response():  response was a success but failed with conversion: {e:?}"
+			);
+			Error::response_decode_error(status.as_u16(), &response_body)
+		})
 	}
 
-	let resp = response
-		.json::<serde_json::Value>()
-		.await
-		.map_err(Error::HorizonResponseError)?;
+	let Ok(resp) = serde_json::from_slice::<serde_json::Value>(&response_body) else {
+		tracing::warn!("interpret_response(): cannot convert error response to json");
+
+		return Err(Error::response_decode_error(status.as_u16(), &response_body));
+	};
 
 	let status =
 		StatusCode::try_from(resp[RESPONSE_FIELD_STATUS].as_u64().unwrap_or(400)).unwrap_or(400);

clients/wallet/src/stellar_wallet.rs

@@ -106,7 +106,12 @@ impl StellarWallet {
 			.pool_idle_timeout(Some(Duration::from_secs(60)))
 			// default is usize max.
 			.pool_max_idle_per_host(usize::MAX / 2)
-			.build()?;
+			.build()
+			.map_err(|e| Error::HorizonResponseError {
+				reqwest: Some(e),
+				status: None,
+				other: None,
+			})?;
 
 		Ok(StellarWallet {
 			secret_key,