Aliheym / typeorm-transactional

A Transactional Method Decorator for TypeORM that uses Async Local Storage or cls-hooked to handle and propagate transactions between different repositories and service methods.
MIT License
219 stars 28 forks source link

Nested transactions don't work as expected #54

Open mdevecka opened 2 months ago

mdevecka commented 2 months ago

original:

try {
  await defaultManager.transaction(async mgr => {
    await mgr.withRepository(repo).insert({ name: "user1" });
    //throw new Error(`abort`);
    await mgr.withRepository(repo).insert({ name: "user2" });
    try {
      await mgr.transaction(async mgr2 => {
        await mgr2.withRepository(repo).insert({ name: "user3" });
        //throw new Error(`abort2`);
        await mgr2.withRepository(repo).insert({ name: "user4" });
      });
    }
    catch (err) {
      // ...
    }
    throw new Error(`abort3`)
  });
}
catch (err) {
  // ...
}

with typeorm-transactional:

try {
  await runInTransaction(async () => {
    await repo.insert({ name: "user1" });
    //throw new Error(`abort`);
    await repo.insert({ name: "user2" });
    try {
      await runInTransaction(async () => {
        await repo.insert({ name: "user3" });
        //throw new Error(`abort2`);
        await repo.insert({ name: "user4" });
      },{ propagation: Propagation.NESTED });
    }
    catch (err) {
      // ...
    }
    throw new Error(`abort3`)
  });
}
catch (err) {
  // ...
}

When using typeorm-transactional test case abort3 does create user3 and user4 which is not desired. (other cases work as expected)

JOOYUNHAK commented 1 month ago

I think it's already committed in the transaction that makes user3, user4

The reason is that if the propagation property is set toNESTED, new transactions that are unrelated to the parent transaction are opened and executed

I think the propagation property should be REQUIRED to throw an error at the abort3 position and don't create user3, user4

mdevecka commented 1 month ago

I was looking for a propagation type with implementation that would not commit nested transactions if parent transaction is aborted. I found some workaround around this in the meantime. Also from your source code there does not seem to be a difference between NESTED and REQUIRES_NEW: https://github.com/Aliheym/typeorm-transactional/blob/master/src/transactions/wrap-in-transaction.ts#L92