-
Notifications
You must be signed in to change notification settings - Fork 285
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
AsyncLocalStorage doesn't propagate across Promise chains? #3041
Comments
@vdeturckheim, @Quard FYI |
@vdeturckheim, @Quard will you be able to weigh in on the above? |
Hey, sorry for the late reply, for some reason I missed the ping. |
So, this is somehow an expected behavior tbh, I slightly changed the code to 'use strict';
'use strict';
const async_hooks = require('async_hooks');
const fs = require('fs');
const util = require('util');
let asyncLocalStorage = new async_hooks.AsyncLocalStorage();
asyncLocalStorage.enterWith({blah: 7});
function log(...args) {
fs.writeSync(1, `${util.format(...args)}\n`);
}
function getCurrentId() {
return async_hooks.executionAsyncId();
}
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
log(`${triggerAsyncId} => ${asyncId}`);
}
}).enable();
log(`before all async id: ${async_hooks.executionAsyncId()}`);
Promise.resolve()
.then(() => {
log(`first promise async id: ${async_hooks.executionAsyncId()}`);
log(`first promise: ${getCurrentId()}`);
log(`before enterWith, id in first promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
asyncLocalStorage.enterWith({id: 7});
log(`id in first promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
log(`first promise done`);
})
.then(() => {
log(`second promise async id: ${async_hooks.executionAsyncId()}`);
log(`second promise: ${getCurrentId()}`);
log(`id in second promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
}); and it logs
Basically, the second promise async scope is created before the first one ran. At this point, they share the same object as strore. When updating the store in the promise, this was not spread because there was no new call to init. That's a bit why I actually recommend to always avoid using (cc @bmeck I think this is one of the cases that concerns you regarding this API). |
correct |
This is expected behaviour. The const als = new AsyncLocalStorage()
als.run({ foo: 'bar' }, () => {
Promise.resolve()
.then(() => {
return als.run({ foo: 'baz' }, () => Promise.resolve())
})
.then(() => {
console.log(als.getStore()) // contains foo: bar
})
}) |
@Qard @vdeturckheim thanks for looking into this! Indeed, this matches my experience that I encountered this when creating tests using I'm tempted to implement my own flavor of Are there caveats here that I'm not anticipating? This is my first time working with |
That sounds reasonable to me. You will just need to make sure you have a way to get the promise resource from the promiseResolve hook so you can propagate the context at that point. It might also make sense as an extra setting for AsyncLocalStorage to propagate in that way, if desired. Just be aware that there is a very specific reason this is not the default behaviour. Allow me to explain. A promise is conceptually not itself async per-se, but rather it can be seen as an abstraction of something else async like an event emitter is sync until you emit from within some other asyncrony. But the fact that a promise handler is scheduled on the microtask queue makes it a form of queueing system. Normally the suggested way to handle function queuing systems is to wrap them with AsyncResource. Due to the low-level nature of promises this is not possible so we instead use PromiseHook which roughly corresponds to the async_hooks lifecycle events. But why does init happen where it does, and why is promiseResolve a separate thing? From the user perspective, AsyncResource ensures a context links back to the user code which supplied the callback and not to internals somewhere such as a connection pool which could potentially trigger from a completely unrelated context. It's the same case here in that what happens within a then handler is internal to the promise whereas the attachment of that then should link back to the context in which it was attached. This problem mostly goes away in async/await as the continuations are effectively nested rather than chained, due to how the spec works, making the outer resolve and the init effectively the same context. The graph produced by async_hooks is not a pure logical graph, it is an abstract conceptual graph, expressing the structure of the program as seen by the user, not as understood by the underlying system which views it purely as a sequence of interleaved events received from the event loop and mapped to callbacks. |
I (think I) get what you mean by The way I'm currently thinking about my use case is that I want a particular piece of context to travel across "scheduling details" such as async/await, Promises, queues, events, etc. Seems like this is in conflict with your description of the way If I understand you correctly, you're saying that As for enhancing (or "forking") 'use strict';
const async_hooks = require('async_hooks');
const fs = require('fs');
const util = require('util');
let asyncLocalStorage = new async_hooks.AsyncLocalStorage();
asyncLocalStorage.enterWith({blah: 7});
function log(...args) {
fs.writeSync(1, `${util.format(...args)}\n`);
}
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
log(`${triggerAsyncId} => ${asyncId}`);
},
promiseResolve(asyncId) {
log(`resolve ${asyncId}`);
}
}).enable();
log(`before all async id: ${async_hooks.executionAsyncId()}`);
Promise.resolve()
.then(async () => {
log(`first promise async id: ${async_hooks.executionAsyncId()}`);
await new Promise((res, rej) => setTimeout(() => res(), 1000)); // <-- creates a new async context
asyncLocalStorage.enterWith({id: 7});
log(`first promise async id: ${async_hooks.executionAsyncId()}`);
log(`id in first promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
log(`first promise done`);
})
.then(() => {
log(`second promise async id: ${async_hooks.executionAsyncId()}`);
log(`id in second promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
}); Now, the second Promise runs in a child context of the first Promise, but I can probably figure out how to deal with this scenario, but this again makes me wary that the real problem here is with what I expect, not with the existing language mechanisms. |
PromiseHook is part of V8. I'm working on a rewrite which should (eventually) enable it to be exposed in JavaScript cleanly, as a separate API, but it's not there yet. Currently it has a partly JavaScript-facing internal API, but only conditionally, so it's not yet possible for it to be used directly. As for context flowing through promises, there's two ways to look at that: what you want is the internal path which flows through the resolving logic, whereas what it does now is step around that, binding the callback directly to the point where then(...) was called. This is important because a promise is a container which can be passed around and then(...) could be run from anywhere. If you were to flow context from the resolve path, that path might be entirely unrelated to the actual user code path, for example, a common pattern in database libraries is to create a connection promise out-of-band from a request, and do a then(...) continuation on it whenever the connection is needed. Flowing from the resolve path in this case would point outside of the request, which would be incorrect. By linking context of a then/catch handler directly to the point where it is called, it ensures the context within that callback is conceptually valid in expressing the relation in user code between the point where the handler is attached and the point where it is run. It's still possible to interact with the stored object if the promise resolution is a direct result of actions with the request, but replacing the object will not flow into the handler because the resolve and the corresponding promise are decoupled with a microtask barrier between when the resolve actually happens and when the handler runs--the handler could theoretically be put off indefinitely even though the resolve has already happened, depending on interactions with the microtask queue. |
To your database example, I understand why one would expect the ALS store to be propagated during I played some more with 'use strict';
const async_hooks = require('async_hooks');
const fs = require('fs');
const util = require('util');
let asyncLocalStorage = new async_hooks.AsyncLocalStorage();
asyncLocalStorage.enterWith({blah: 7});
function log(...args) {
fs.writeSync(1, `${util.format(...args)}\n`);
}
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
log(`${triggerAsyncId} => ${asyncId} (${type})`);
},
promiseResolve(asyncId) {
log(`resolve ${asyncId}`);
},
after(asyncId) {
log(`after ${asyncId}`);
},
destroy(asyncId) {
log(`destroy ${asyncId}`);
},
}).enable();
log(`before all async id: ${async_hooks.executionAsyncId()}`);
Promise.resolve()
.then(async () => {
// runs in context id 3
log(`first promise async id: ${async_hooks.executionAsyncId()}`);
await new Promise((res, rej) => setTimeout(() => res(), 1000));
// runs in context id 8 (grandparent: 3)
Promise.resolve()
.then(() => {
// runs in context id 11 (grandparent: 8)
log(`subpromise: ${async_hooks.executionAsyncId()}`);
});
// still runs in context id 8 (grandparent: 3)
asyncLocalStorage.enterWith({id: 7});
log(`first promise async id: ${async_hooks.executionAsyncId()}`);
log(`id in first promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
log(`first promise done`);
})
.then(() => {
// runs in context id 4 (parent: 3)
log(`second promise async id: ${async_hooks.executionAsyncId()}`);
log(`id in second promise=${asyncLocalStorage.getStore().id} and blah=${asyncLocalStorage.getStore().blah}`);
}); I get the following output:
So now I set my store in the first Promise, but in a context that's a grandchild of the Promise context. That context is not a parent of the context of the second Promise. Rather, they are siblings (or even cousin and uncle...). So this means I have to do a more sophisticated relationship-analysis when deciding where I need to propagate the store upon To complicate things further, I wouldn't expect store updates done within the sub-Promise to propagate to the second Promise, but I can't come up with a good mechanism for ensuring this, without relying on the exact context parent/child tree which seems incredibly fragile. I thought that maybe the It seems like the only thing to do at this point is to keep in mind the potential pitfalls and use the API very carefully. I'm also looking at https://www.npmjs.com/package/async-local-storage, which is similar to |
hey @yairk any update about the issue? We are experience the same in node 16.17 and jest. If we init |
@ezekiel25-17 we did not find a way to do this purely with |
Hi everyone 👋 I've been running into this kind of scenario with Here are two examples:
Maybe through your testing, someone could shed some light on why the second one acts like this. Note that both examples are run using Node 16.18.0 without any transpiler (which I initially thought could be messing with the compiled code). First exampleconst { AsyncLocalStorage } = require('async_hooks');
const als = new AsyncLocalStorage();
/** Runs the main code and reads the state of the AsyncLocalStorage after execution */
async function main() {
console.log('main 1, store:', als.getStore());
await doSomething();
console.log('main 2, store:', als.getStore());
}
/**
* Sets the AsyncLocalStorage value with `enterWith` for the remainder of the current synchronous execution
* https://nodejs.org/api/async_context.html#asynclocalstorageenterwithstore
*/
function prepareALS(value) {
als.enterWith(value);
}
/** Play with the store */
async function doSomething() {
console.log('doSomething 1, store:', als.getStore());
prepareALS(1);
console.log('doSomething 2, store:', als.getStore());
prepareALS(2);
console.log('doSomething 3, store:', als.getStore());
prepareALS(3);
console.log('doSomething 4, store:', als.getStore());
}
main()
.then(() => console.log('done'))
.catch((err) => console.error(err)); Resulting logs: main 1, store: undefined <= expected, no value set yet
doSomething 1, store: undefined <= expected, no value set yet
doSomething 2, store: 1 <= expected, value just set to 1 in the current context
doSomething 3, store: 2 <= expected, value just set to 2 in the current context
doSomething 4, store: 3 <= expected, value just set to 3 in the current context
main 2, store: 3 <= expected, value was lastly set to 3 in the synchronous context
done Second exampleNotice that only the const { AsyncLocalStorage } = require('async_hooks');
const als = new AsyncLocalStorage();
/** Runs the main code and reads the state of the AsyncLocalStorage after execution */
async function main() {
console.log('main 1', als.getStore());
await doSomething();
console.log('main 2', als.getStore());
}
/**
* Sets the AsyncLocalStorage value with `enterWith` for the remainder of the current synchronous execution
* https://nodejs.org/api/async_context.html#asynclocalstorageenterwithstore
*/
async function prepareALS(value) {
als.enterWith(value);
}
/** Play with the store */
async function doSomething() {
console.log('doSomething 1', als.getStore());
await prepareALS(1);
console.log('doSomething 2', als.getStore());
await prepareALS(2);
console.log('doSomething 3', als.getStore());
await prepareALS(3);
console.log('doSomething 4', als.getStore());
}
main()
.then(() => console.log('done'))
.catch((err) => console.error(err)); Resulting logs: main 1, store: undefined <= expected, no value set yet
doSomething 1, store: undefined <= expected, no value set yet
doSomething 2, store: 1 <= expected, value just set to 1 in the current context
doSomething 3, store: 2 <= expected, value just set to 2 in the current context
doSomething 4, store: 3 <= expected, value just set to 3 in the current context
main 2, store: 1 <= ?! unexpected
done So at the end of the second example, I see two possible coherent results:
However, the ALS ends up keeping the value set in the first asynchronous call for some reason that I can't explain. |
You changed a bit more then Only the call to |
Hi @Flarna. Thanks for the quick reply and the explanation, that makes sense 👍 So I've finally managed to reach the state that I wanted through a "simple" workaround and ensure the last value set on the AsyncLocalStorage, even in a further Promise, is properly readable when back in the main sync context. The idea is simply to always bind the execution context of a function that gets the AsyncLocalStorage store to the latest context that set the store value via Let me know if you see anything that could go wrong with this method, any comments are welcome 🙂 const { AsyncLocalStorage, AsyncResource } = require('async_hooks');
const als = new AsyncLocalStorage();
/** Runs the main code and reads the state of the AsyncLocalStorage after execution */
async function main() {
console.log('main 1, store:', getLatestStore());
await doSomething();
console.log('main 2, store:', getLatestStore());
}
/** Our mutable function allowing to get the ALS store in the last context that it was set */
let getLatestStore = getStore;
function getStore() {
return als.getStore();
}
/**
* Sets the AsyncLocalStorage value with `enterWith` for the remainder of the current synchronous execution
* https://nodejs.org/api/async_context.html#asynclocalstorageenterwithstore
*/
async function prepareALS(value) {
als.enterWith(value);
// Set `getLatestStore` to return the store in the current context
getLatestStore = AsyncResource.bind(getStore);
}
/** Play with the store */
async function doSomething() {
console.log('doSomething 1, store:', getLatestStore());
await prepareALS(1);
console.log('doSomething 2, store:', getLatestStore());
await prepareALS(2);
console.log('doSomething 3, store:', getLatestStore());
await prepareALS(3);
console.log('doSomething 4, store:', getLatestStore());
}
main()
.then(() => console.log('done'))
.catch((err) => console.error(err)); Resulting logs: main 1, store: undefined
doSomething 1, store: undefined
doSomething 2, store: 1
doSomething 3, store: 2
doSomething 4, store: 3
main 2, store: 3
done |
Honestly speaking I don't know the use case here. Usually using I assume in your final setup I also don't understand why |
I would recommend against using |
I filed nodejs/node#45848 which is basically a dup of this issue, but there is one little wrinkle I don't see called out in the comments above. While I get that Consider these examples where the inner async context behavior changes depending on whether other stores are in play and if you use async/await syntax vs. promises:
Because the behavior changes depending on seemingly unrelated ALS stuff, in Jest land this can manifest itself as test suites that pass/fail depending on whether they are run in isolation. |
It seems there has been no activity on this issue for a while, and it is being closed in 30 days. If you believe this issue should remain open, please leave a comment. |
It seems there has been no activity on this issue for a while, and it is being closed. If you believe this issue should remain open, please leave a comment. |
I've been playing with
AsyncLocalStorage
with the purpose of using it for propagating context across web requests. I had issues getting it to work in Jest-based unit-tests: specifically, stores set inbeforeEach
would not propagate to the tests. Ultimately I realized that the test's async context was created prior to thebeforeEach
hook being actually run.While trying to investigate this, I came up with the following code:
With some debug prints enabled (courtesy of async-local-storage), the above code gives the following output:
Am I missing something in expecting the store to propagate to the third promise as well?
The text was updated successfully, but these errors were encountered: