501 fix occassional horizonresponse decodeerror (#502)

* Run `cargo fmt --all`

* Implement resubmission with fee bump transaction

* Replace fee-bump transaction with normal transaction

* Change log level

* Change other test

* Bound tx fee to a maximum

* add logs for response

* update structure of `HorizonResponseError`

* update structure of `HorizonResponseError`

* cleanup dupplicates

* recover if the error is recoverable

* #502 (comment),
#502 (comment)

* revert code

---------

Co-authored-by: Marcel Ebert <[email protected]>

clients/wallet/src/error.rs

@@ -8,8 +8,8 @@ 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: error: {error:?}, other: {other:?}")]
+	HorizonResponseError { error: 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 +43,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, error, .. } => {
+				if let Some(e) = error {
+					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 +76,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, error, .. } => {
+				if let Some(e) = error {
+					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 { error: 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 {
+			error: 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 { error: 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,27 @@ 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();
+
+	let response_body = response.bytes().await.map_err(|e| {
+		tracing::warn!("interpret_response(): cannot convert response to bytes: {e:?}");
+		Error::HorizonResponseError { error: 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 {
+				error: Some(e),
+				status: None,
+				other: None,
+			})?;
 
 		Ok(StellarWallet {
 			secret_key,