keystone icon indicating copy to clipboard operation
keystone copied to clipboard

Transaction should prevent after operation hooks until committed

Open gautamsi opened this issue 1 year ago • 1 comments

  • Create a transaction top update/create an item in a list which has afterOperation side effects causing action outside of transaction like queue postage label purchase based on order update
  • rollback the transaction just before completion
  • see the side effects still working while the actual transaction is rolled back

I should be seeing no afterOperation hook until the transaction is committed.

I believe this is debatable if the side effects should be allowed or not while transaction may be rolled back

Do we have a way to know if the afterOperation is running in transaction or not? at least I can try to defer the side effects somehow.

gautamsi avatar Aug 26 '24 19:08 gautamsi

@gautamsi this is intended, and arguably the only reasonable option when you have nested operations.

The underlying problem is that we have emphasised that beforeOperation and afterOperation should be used for out-of-band side effects; while simultaneously encouraging their usage for scenarios that should be wrapped in a transaction.

We cannot reasonably put beforeOperation hooks outside of a transaction; as the nested operations within a transaction cannot be easily known up front. And while we could queue up afterOperation hooks to after the transaction, this will break for users who expect their afterOperation hooks fire directly _after the intended operation, within the transaction.

When sparring about this with @molomby, we thought maybe a new hook that is equivalent to an afterCommit and or afterRollback could be appropriate; but fell short of adding this in the prototype as a result of not wanting to complicate that experiment before collecting feedback (like this).

hooks: {
  transaction: {
    commit: () => {},
    rollback: () => {}
  }
}

If the relevant item operation has not been executed within a transaction context, we could optionally decide if commit should always be called as an equivalent to afterOperation.

dcousens avatar Aug 27 '24 00:08 dcousens