aws / aws-dax-go

AWS DAX SDK for the Go programming language. https://aws.amazon.com/dynamodb/dax
Apache License 2.0
47 stars 48 forks source link

Missing support for transaction cancelation reasons #14

Closed ccbrown closed 4 years ago

ccbrown commented 4 years ago

As far as I can tell, it's not possible to get cancelation reasons for transactions.

DAX does send them to clients, and the library does parse them, but it doesn't appear to use or expose them in any way:

https://github.com/aws/aws-dax-go/blob/53d8d79dad719fa9cccd9403ae09bc751fb61ac2/dax/internal/client/error.go#L41

Would it be possible to expose these?

For what it's worth, accessing cancelation reasons is possible in aws-sdk-go, albeit pretty painful: https://github.com/aws/aws-sdk-go/tree/3c8d61f5257e0e0da3e8d901f56c6765878f09e5/example/service/dynamodb/transactWriteItems

So if a proper API is blocked by aws-sdk-go support, maybe it would be reasonable to allow users to override decodeError in a way similar to aws-sdk-go?

VasilyFomin commented 4 years ago

The error will eventually be reported to the client, unless the request succeeded after retry, in which case last error will be nil:

https://github.com/aws/aws-dax-go/blob/19b34c5c99ea4b76610dcbae7d87fd572de012df/dax/internal/client/single.go#L642-L643

or would you like to see errors for each attempt, which I think is not possible at the moment?

ccbrown commented 4 years ago

The error is reported, but the complete reason is not accessible to the client because there's no way to access the private cancellationReasonCodes, cancellationReasonItems, etc. fields. So if you do an atomic write that has two conditions, and one fails, you have no way of knowing which one of the two conditions failed.

If daxTransactionCanceledFailure were made public or exposed its fields through a public interface, that would solve this problem. In fact, whenever I need this functionality, I use a fork that exposes these fields with just a few small additions to the codebase: https://github.com/theaaf/aws-dax-go/commit/5c383f7367adc2cf89bb13923856de920672a3b7

It would be really nice if there was a way to get access to these fields without forking.

dmartin1 commented 4 years ago

I'm in the same position. Having access to the underlying CancellationRasonCodes would be extremely useful, and better shape the response of the method in several cases. Any chance this is on the roadmap for a future release?

ccbrown commented 4 years ago

An API for this was added in aws-sdk-go: https://github.com/aws/aws-sdk-go/releases/tag/v1.28.0

See the example here: https://github.com/aws/aws-sdk-go/blob/0ec921513a4536a315d4dd89416fc746b786ca2e/service/dynamodb/cust_example_test.go#L10

So this is no longer just a missing feature; it's now also a compatibility issue as error handling code written for the SDK will break when used with this library (but maybe that's a different issue since the SDK has a lot of exception types that this library doesn't use).

lyaoxion commented 4 years ago

Thanks @ccbrown for mentioning that aws-sdk-go now has new public interface to access CancellationReasons in a TransactionCanceledException. We are now working on implementing that in this library.

lyaoxion commented 4 years ago

Transaction cancellation reasons are now available in v1.2.0 through asserting the error to dynamodb.TransactionCanceledException.