Nested transactions can't access records created in parent transactions

[santiagodoldan]

What are you doing?

// I'm using `cls-hooked`

sequelize.transaction(async () => {
  const user = await User.create(...)

  console.info(await User.findOne({ where: { id: user.id } })) // returns the user

  await sequelize.transaction(async () => {
    console.info(await User.findOne({ where: { id: user.id } })) // returns null
  })
})

What do you expect to happen?

I was expecting the same result as Rails where nested transactions can find records created in parent transactions.

Dialect: postgres Database version: 9.5.2 Sequelize version: 4.10.0 Tested with master branch: Yes

[santiagodoldan]

Any ideas if I'm doing something wrong?

[jprestel-rue]

Possibly 2 things are happening:

1. Based on your description of the problem, I'm assuming you're using CLS to auto-pass the transaction to User.create.
   • If you weren't, I assume the create would be committed immediately and then would be visible to the second transaction
2. You have to explicitly tell the second transaction that it has a parent transaction. as written, you simply have 2 concurrent, but unrelated transactions.
   • Even with CLS, transactions aren't automagically nested. You have to be explicit here.
   • This is actually a feature my team would like, so I'm going to open a PR for it shortly 🙃

You can address number 2 by passing the transaction as an option.

sequelize.transaction(async (transaction) => {
  const user = await User.create(...)

  console.info(await User.findOne({ where: { id: user.id } })) // returns the user

  await sequelize.transaction({ transaction }, async () => {
    console.info(await User.findOne({ where: { id: user.id } })) // returns null
  })
})

To better understand this, take a look at the source for the transaction constructor: https://github.com/sequelize/sequelize/blob/68d729d87963dc95184945f03ed5120b3cfa30c2/lib/transaction.js#L37-L48

[santiagodoldan]

@jprestel-rue yep, we'd like to avoid having to explicitly pass transactions all around the project, that's why we added cls. if you have a PR and need some help testing it, please let me know 👍

[santiagodoldan]

Any news here? @jprestel-rue

[jprestel-rue]

Been completely under water with unrelated work, unfortunately. So, while this is still on my to do list, it's sunk down in priority 🙁

That said, if you wanted to take this on, my idea for a "fix" was to create a new optional parameter on the transaction constructor that, if set to true, would automatically assign the current transaction as the new transaction's parent. This parameter would default to false to maintain backwards compatibility.

[santiagodoldan]

@jprestel-rue will try to play a bit with this during the weekend 💪 let's see if I can get something done

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂