Raise exception on errors

[rwdaigle]

Like we do in Gala, we should raise an error when known error states are encountered. Looking at the lib it looks like one such state is when the mac verification fails.

Perhaps DigestVerificationError?

[mrezentes]

I made errors for 3 foreseeable error scenarios -

https://github.com/spreedly/r2d2/blob/master/lib/r2d2/payment_token.rb#L10

https://github.com/spreedly/r2d2/blob/master/lib/r2d2/payment_token.rb#L65

https://github.com/spreedly/r2d2/blob/master/lib/r2d2/payment_token.rb#L32

[mrezentes]

@rwdaigle ruby code nitpicking requested for those errors.

[rwdaigle]

Great! Couple design points:

This library should have one job - to decrypt the payment token. So, we should do as little else as possible, including modifying the decrypted keys. Whatever the keys/values are in the token, we should just return it and not make any assumptions about key naming.

If we remove the key renaming, I believe we will also be able to remove the need to do any JSON.parse or JSON.generate. Just return whatever is decrypted and let the caller decide how to parse. This is especially important because, in Ruby, there are many JSON parsing libraries offering various pros/cons, so keeping this concern outside the scope of the library gives the caller more flexibility.

Also, looking at DecryptionError, I think we should let whatever the underlying exception is just bubble up. Catching an exception and re-typing it like that only serves to obscure the original error context (message, line number, stack trace, etc...).

If we do the above, I think we remove the need for DecryptedMessageUnparseableError and DecryptionError.

[mrezentes]

Exactly the type of feedback I was looking for. Thanks.