ruby-stellar-sdk Raise useful Horizon errors

I caught this on the side while working on something else, my apologies for the missing tests; I didn't have time to figure out how to set up the accounts correctly just yet.

I ran into Horizon rejecting the XDR envelopes made from a regular send_payment request. The exception raised was as such:

     Faraday::BadRequestError:
       the server responded with status 400
     # ./lib/account.rb:20:in `send_payment'

Which is not exactly helpful. Which is why I'm humbly proposing to wrap those into exceptions ourselves to raise some more useful information. It would now become:

     Stellar::HorizonError::TransactionMalformed:
       Horizon could not decode the transaction envelope in this request. A transaction should be an XDR TransactionEnvelope struct encoded using base64.  The envelope read from this request is echoed in the `extras.envelope_xdr` field of this response for your convenience.

# or

     Stellar::HorizonError::TransactionFailed:
       The transaction failed when submitted to the stellar network. extras.result_codes contained: `{"transaction"=>"tx_failed", "operations"=>["op_no_destination"]}`  Descriptions of each code can be found at: https://www.stellar.org/developers/guides/concepts/list-of-operations.html

It also makes available .extras on the exception. (On further thought, original_hash should probably be named original_response).

[...]

After being done, I saw there were already Stellar::AccountRequiresMemoError and Stellar::Horizon::Problem, we could probably clean this up together if you'd like.

Cheers!

Jun 06 '20 23:06 joallard

Hi, thank you for the PR! Unfortunately, we did a mistake during the recent stellar-base/stellar-sdk merge which resulted in commit history for the base gem completely missing, and in order to fix this mistake we've had to force-push the master (and I did it just now). Could you please rebase your PR against the updated master? The new master should be identical to the old one as far as the repository content is concerned, it's just a commit history which changed, so your patches should apply perfectly. Really sorry about this mess.

Also, I would suggest separating the bug fix and the error handling into different PRs. We definitely want to fix the base64 encoding bug for the upcoming release, but since we're in the rc1 stage already, I'm not sure if we should go ahead and include the error decoding part right now (also, as you pointed out, it needs some cleanup in regards to Stellar::Horizon::Problem/Stellar::HorizonError duplication).

Jun 07 '20 00:06 nebolsin

Of course! Done. The Base64 fix is now in #96.

A simple test could take the form:

When I send a payment to an uncreated account
Then I get a Stellar::HorizonError::TransactionFailed error

Jun 07 '20 00:06 joallard

I saw there were already Stellar::AccountRequiresMemoError and Stellar::Horizon::Problem, we could probably clean this up together if you'd like

I think we can get rid of Stellar::Horizon::Problem, it seems not used anywhere.

Stellar::AccountRequiresMemoError doesn't come straight from Horizon, it's more of application-level error, so I believe we can keep it with Stellar::HorizonError separately