Skip to content

Bug: transaction items are reordered by PynamoDB but order is important #1284

@lehotskysamuel

Description

@lehotskysamuel

Hi,
I'd like to report a bug.

Order of transaction items in a transaction request is important for error handling. AWS docs say:

Transaction cancellation reasons are ordered in the order of requested items, if an item has no error it will have None code and Null message.

However, current pynamodb implementation reorders them:

# excerpt from connection/base.py

    def transact_write_items(
        self,
        condition_check_items: Sequence[Dict],
        delete_items: Sequence[Dict],
        put_items: Sequence[Dict],
        update_items: Sequence[Dict],
        client_request_token: Optional[str] = None,
        return_consumed_capacity: Optional[str] = None,
        return_item_collection_metrics: Optional[str] = None,
    ) -> Dict:
        """
        Performs the TransactWrite operation and returns the result
        """
        transact_items: List[Dict] = []
        transact_items.extend(
            {TRANSACT_CONDITION_CHECK: item} for item in condition_check_items
        )
        transact_items.extend(
            {TRANSACT_DELETE: item} for item in delete_items
        )
        transact_items.extend(
            {TRANSACT_PUT: item} for item in put_items
        )
        transact_items.extend(
            {TRANSACT_UPDATE: item} for item in update_items
        )

As you can see, pynamo sends all condition check items first, followed by all delete items, followed by all put items etc.

This should be easily fixed by not using delete_items, put_items etc as separate arrays - instead you could replace it with a single array that you map to transact_items in this method.

Why is this a problem?

We have two conditions in a single transaction and we want to approach both exceptions differently. One exception signifies "invalid state" and we'd like to raise it. The other exception is something we'd like to recover from with additional business logic.

            try:
                with TransactWrite(connection=pynamo_low_level_connection) as tx:
                    tx.update(
                        debited_account,
                        actions=[Account.credits.balance.add(-amount)],
                        condition=(
                            (Account.credits.balance == old_balance)
                            & (
                                Account.credits.balance > 0
                            )  # Checks the balance is as expected (prevents race conditions)
                        ),
                    )
                    tx.save(spending_transaction, condition=(SpendingTransaction.unique_condition()))

                debited_account.refresh()
                spending_transaction.refresh()

            except TransactWriteError as e:
                # if condition 1 fails, raise
                # if condition 2 fails, recover

Notice we update first and save second, so the causes arrive in a reversed order from the API.

With knowledge of pynamodb implementation, we can implement what we want, so we have a workaround. But it requires knowledge of the pynamo's internal implementation and is not how AWS APIs work. This is confusing for developers.

I believe this should work as AWS APIs do but note that fixing this will be backwards-incompatible and probably deserves a mention in release notes.

Thanks, all the best!


Related to: #1143

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions