Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hooks do not get CLS transaction #12973

Closed
2 of 7 tasks
heroic opened this issue Jan 24, 2021 · 12 comments · Fixed by #13927
Closed
2 of 7 tasks

hooks do not get CLS transaction #12973

heroic opened this issue Jan 24, 2021 · 12 comments · Fixed by #13927
Labels
released on @v7 topic: cls For issues and PRs. Everything that involves CLS type: bug

Comments

@heroic
Copy link
Contributor

heroic commented Jan 24, 2021

Issue Description

If using CLS to set a transaction, then the transaction is not received in the hooks.

What are you doing?

We set a transaction on the HTTP request using CLS. We expect the transaction to be available inside the hooks to be able to use the afterCommit transaction hook. However, options does not contain the transaction.

Here is the link to the SSCCE for this issue: LINK-HERE

This is inside a Koa2 app.

app.use(async (_, next) => {
  await sequelize.transaction(() => next());
});

In the model hook

beforeSave: (instance, options) => {
  console.warn(options.transaction);
},

The output is undefined.

What do you expect to happen?

options.transaction should be the same as the transaction set in the CLS.

What is actually happening?

options.transaction is undefined

undefined

Additional context

It appears that this logic is missing inside save:

if (options.transaction === undefined && this.sequelize.constructor._cls) {

Adding this to save fixes this issue.

Environment

  • Sequelize version: 6.4.0
  • Node.js version: 12.20.0
  • Operating System: Mac OSX

Issue Template Checklist

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.
  • I think this problem happens only for the following dialect(s):
  • I don't know, I was using PUT-YOUR-DIALECT-HERE, with connector library version XXX and database version XXX

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start, but I don't know if this was left out intentionally
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
@heroic
Copy link
Contributor Author

heroic commented Jan 24, 2021

Also discussed here #8585

@heroic
Copy link
Contributor Author

heroic commented Jan 29, 2021

@papb could you take a look at this? I can raise a PR for this, is this is a miss and there is no real reason to have skipped passing the transaction to the hooks

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Oct 27, 2021
@heroic
Copy link
Contributor Author

heroic commented Oct 28, 2021

not stale

@heroic
Copy link
Contributor Author

heroic commented Oct 28, 2021

@papb Any updates here?

@github-actions github-actions bot removed the stale label Oct 29, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2021

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Nov 6, 2021
@heroic
Copy link
Contributor Author

heroic commented Nov 7, 2021

not stale

@github-actions github-actions bot removed the stale label Nov 8, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has been open for 14 days without activity. It will be closed if no further activity occurs within the next 14 days. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Nov 23, 2021
@heroic
Copy link
Contributor Author

heroic commented Nov 23, 2021

not stale

@github-actions github-actions bot removed the stale label Nov 24, 2021
@WikiRik WikiRik added topic: cls For issues and PRs. Everything that involves CLS type: bug labels Nov 26, 2021
@WikiRik
Copy link
Member

WikiRik commented Nov 26, 2021

Sorry about the late response, it would be nice if you could make a PR for this @heroic

@heroic
Copy link
Contributor Author

heroic commented Jan 9, 2022

@WikiRik Added!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2022

🎉 This issue has been resolved in version 7.0.0-alpha.15 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @v7 topic: cls For issues and PRs. Everything that involves CLS type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants