501 fix occassional horizonresponse decodeerror

[b-yap]

closes #501

The error found was 403, and as explained by @ebma, it should be recoverable.
Not only that, I've updated the HorizonResponseError structure, flexible enough to accept either a ReqwestError or an error in string format.

The string format is important to find the correct error.

[ebma]

Looks good to me 👍 I'm curious if this fixes the problem, hard to tell. @b-yap did you happen to test this locally? Encountering these timeouts is probably hard to reproduce locally, as you would need to simulate some redeem requests.

[b-yap]

@ebma by flagging this error as "recoverable", it will resend the same transaction again.
see https://github.com/pendulum-chain/spacewalk/blob/501-fix-occassional-horizonresponse-decodeerror/clients/wallet/src/horizon/horizon.rs#L167.

[ebma]

Yes, the logic makes sense to me. I was asking if you tested it because it would actually be interesting to see how this behaves in practice. Because if this error is indeed somehow related to rate limiting, our logic here is not enough to make the transaction eventually go through. Since even if we retry with the same transaction over and over again, the rate-limiting would continue to block it. If this is the case, which I don't know, then we should add some kind of delay here and wait for the rate-limiting window to expire.

[b-yap]

Yesterday it was difficult with the accounts not existing.

There is already a delay added on the loop, the backoff_delay_counter.

spacewalk/clients/wallet/src/horizon/horizon.rs

Lines 177 to 183 in a2624f0

	let sleep_duration = backoff_delay_counter * BASE_BACKOFF_DELAY_IN_SECS;
	sleep(Duration::from_secs(sleep_duration)).await;
	// retry/resubmit again
	if sleep_duration < u64::from(max_backoff_delay_in_secs) {
	backoff_delay_counter += 1;
	}

Will this slowdown the CI? Possible.

[ebma]

Ahhh true, we already have that delay in place 👍 That's great.

I found out that the Horizon's apparently have IP-based rate limiting of 3600 requests per hour ie. one request per second on average.

[ebma]

Let's give these changes a try in production. In any case they won't do any harm and at least improve the decoding of some horizon responses, even if we are not 100% sure whether submissions that returned a 403 error will eventually get accepted or not.

[ebma]

Only one integration test failed so we can consider it a shaky test. I checked cargo clippy and rustfmt locally and there are no complaints. Merging.