more correctly and comprehensively cover retry cases for both version…

…ed_diffed_update_item and versioned_transact_write_items
xoeye · May 6, 2021 · 7e392e9 · 7e392e9
1 parent b688e00
commit 7e392e9
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 17 deletions.
diff --git a/tests/xoto3/dynamodb/write_versioned/run_test.py b/tests/xoto3/dynamodb/write_versioned/run_test.py
@@ -39,7 +39,16 @@ def transact_write_items(TransactItems: List[dict], **kwargs):
         attempts += 1
         if attempts == 1:
             raise ClientError(
-                {"Error": {"Code": "TransactionCanceledException"}}, "transact_write_items"
+                {
+                    "Error": {
+                        "Code": "TransactionCanceledException",
+                        "CancellationReasons": [
+                            dict(Code="ConditionalCheckFailed"),
+                            dict(Code="ProvisionedThroughputExceeded"),
+                        ],
+                    }
+                },
+                "transact_write_items",
             )
         # otherwise "succeed"
 
@@ -82,7 +91,7 @@ def build_transaction(tract):
     # we eventually raise after lots of failures
     def transact_always_cancel(TransactItems):
         raise ClientError(
-            {"Error": {"Code": "TransactionCanceledException"}}, "transact_write_items"
+            {"Error": {"Code": "TransactionInProgressException"}}, "transact_write_items"
         )
 
     with pytest.raises(TransactionAttemptsOverrun) as e:

diff --git a/xoto3/dynamodb/write_versioned/ddb_api.py b/xoto3/dynamodb/write_versioned/ddb_api.py
@@ -28,13 +28,6 @@
 
 _log = getLogger(__name__)
 
-_RetryableTransactionCancelledErrorCodes = {
-    "ConditionalCheckFailed",
-    "TransactionConflict",
-    "ThrottlingError",
-    "ProvisionedThroughputExceeded",
-}
-
 
 _DDB_RES = tll_from_session(lambda s: s.resource("dynamodb"))
 _DDB_CLIENT = tll_from_session(lambda s: s.client("dynamodb"))
@@ -80,18 +73,21 @@ def _collect_codes(resp: dict) -> Set[str]:
     return cc
 
 
-_KNOWN_RETRYABLE_TRANSACTION_ERRORS = (
-    "TransactionCanceledException",
-    "TransactionInProgressException",
-)
+_RetryableTransactionCancelledErrorCodes = {
+    "ConditionalCheckFailed",
+    "TransactionConflict",
+    "ThrottlingError",
+    "ProvisionedThroughputExceeded",
+}
 
 
 def _is_transaction_failed_and_retryable(ce: ClientError) -> bool:
     error_name = client_error_name(ce)
-    return (
-        error_name == "TransactionCanceledException"
-        and _collect_codes(ce.response) <= _RetryableTransactionCancelledErrorCodes
-    ) or error_name == "TransactionInProgressException"
+    if error_name == "TransactionInProgressException":
+        return True
+    if error_name == "TransactionCanceledException":
+        return _collect_codes(ce.response) <= _RetryableTransactionCancelledErrorCodes
+    return False
 
 
 def is_cancelled_and_retryable(ce: ClientError) -> bool: