-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
continuation-local-storage support #583
Comments
Why doesn't CLS use domains? They are already enabled in bluebird. In any case, only a generic mechanism like domains is supported, not a specific implementation like CLS. |
CLS is a general mechanism for storing local variables in callback / promise chains (advertised as thread-local storage for callback chains), whereas domains only support error handling as far as I know. However, I understand the sentiment of not wanting to litter bluebird with implementation details about other libraries, and shimming in sequelize is not too much work :) |
That's not what I mean, domains are also used to implement libraries like CLS: var d = domain.create();
d.run(function() {
domain.active.data = {foo: 1};
setTimeout(function() {
domain.active.data.foo // 1;
}, 10);
});
setTimeout(function() {
// Throws an error
domain.active.data.foo
}, 10); There is nothing generic about CLS, it just monkey patches async methods it knows about hence it's not working with bluebird. Domains work the same way but at least they are in the core so they are supported in bluebird: var d = domain.create();
d.run(function() {
domain.active.data = {foo: 1};
Promise.delay(10).then(function() {
domain.active.data.foo // 1;
});
});
Promise.delay(10).then(function() {
// Rejects from the type error
domain.active.data.foo
}); |
I was not aware you could do that with domains. Thank you for your input! |
Hi,
I opened sequelize/sequelize#3509 in the Sequelize project and, long story short, the main issue is that they had to shim Bluebird to support continuation-local-storage.
Instead of shimming the global Bluebird lib, they shimmed their own instance. This was a design decision as discussed on the thread with @mickhansen and @janmeier.
You might be wondering why on earth I am discussing Sequelize and CLS issue in a Bluebird issue. Well, that discussion made me wonder why CLS support couldn't be backed in bluebird. CLS is so nice for use cases like transaction, log context, HTTP request storage, ...
API could be something like
Bluebird.enableCLSNamespace(ns)
and implementation inspired by https://github.com/TimBeyer/cls-bluebird/blob/master/shim.js but without having to resort to shimming.It's totally fine with me if your think this is a stupid idea and/or not in the scope of this project. I just want to know if you'd be open to such feature.
The text was updated successfully, but these errors were encountered: