TC39 Async Context on 2024-05-19

13:07	<Stephen Belanger>	Here's a new issue from Matteo about the exact issue I'm talking about with await binding. I've seen this user confusion come up a lot. https://github.com/nodejs/node/issues/53037
13:08	<Stephen Belanger>	It happens to use `enterWith(...)` rather than `run(...)`, but the confusion around expected flow is effectively the same.
13:26	<Stephen Belanger>	Do you have an example of this sort of fix, and the costs and benefits of it, that we can look into? We use it all over the place in the Datadog tracer, though we've been gradually migrating away to doing context recovery using TracingChannel instead as we don't break other users and tracing products that way. The cost is not really much, and like I said will happen whether we provide fast-paths for it or not--you can just do `store.get()` and `store.run(...)` in a closure, but then you're making a closure and doing some extra steps which could probably be more optimizable as an instance-scoped bind method of some sort.
13:28	<Stephen Belanger>	Catching up on a lot here. I think I'm starting to come around a bit more to Stephen's perspective w.r.t. context flowing out from resolves. I think he's right that, in general (when people do the right thing) it ends up maintaining the same root for the context tree, since the resolved promise generally comes from earlier in the same scope, and I like the fact that it aligns the opt-in for registration time with something that's actually quite reasonable to implement. I believe it's also pretty trivial to do a paranoid await-wrapping: `await bindTask(() => untrustedApi())` which would guarantee the untrustedApi can't change the context on you. Where it still feels wrong to me is Andreu's concern. The really nice property in the current proposal is that it's really well encapsulated. Context variable behave just like lexical consts, where you have guarantees that anything you can't see can never change them out from under you, and that's a _very attractive_ guarantee. Whereas the flows-out approach seems very brittle if any single bad/careless actor anywhere in your downstream call chain is able to irretrievably break the flow. I think that's where this disconnect is coming from - the encapsulation purists in the group are very hesitant to give up that guarantee. I wonder if there's some middle ground where you could at least detect when an abrupt context change has occurred? For instance, I could imagine something along the lines of `using _ = contextMonitor();` at the top of the function. It could install a new variable and if it detected that the variable has changed at the end of the scope, it knows something fishy has happened. And if we're giving up the encapsulation, I suspect mutating variables with `using` might actually be reasonable as well... The bad actor changing your value is only a problem if you explicitly give them the store and let them do that. If you just keep your stores private this is not a real problem.
13:33	<Stephen Belanger>	Also, the `using` syntax doesn't play particularly nice with async context as it not only crosses over async barriers, but also (as far as I'm aware) does nothing to signal any sort of change of state around awaits in its scope so if you, for example, mutate a global in whatever the `using` is doing and then expect it to restore the value when the use expires it may also be required that the value is altered to match the appropriate value between async code, so I expect that is going to be a bit of a footgun when combined with async code.
13:36	<Stephen Belanger>	If you are coming around to this point, do you have ideas about answers to the questions I was asking? Namely, what do we hope to get from merge points, which happen all the time? I have less of a specific intuition on what to do about merge points, though as I have expressed previously I care a lot less about what merges look like as that just produces a mildly incorrect execution flow graph while the await binding produces a very incorrect flow graph which we have to do a bunch of work to patch around, which I described earlier with needing to store everything in a whole request trace and essentially guess from which thing the current path is a continuation.
13:47	<Stephen Belanger>	This per-instance bind sounds like a requirement after the behavior is defined as flowing out to the outer scope. In the current form, this example would not be a problem: `async function someFunctionYouCareAboutTracing() { const response1 = await fetch(...); await someAPI(); // the current context will not be modified by this call const response2 = await fetch(...); return await doSomethingWith(response1, response2); }` So I would say the generally encouraged way to do binds should be instance-scoped by default and global bind should only ever be a "Are you sure you know what you're doing?" type of API for the power-user cases like module authors making sure their resource pool will not leak implementation details that would never be relevant to user code execution flow. Pool mechanisms I would say are almost universally okay to bind globally, but almost every other scenario is a matter of opinion and should (at least in my opinion) probably not bind at all by default and always follow that path through internals because otherwise you end up with these strange flows like with async/await not flowing through awaits the way most users seem to expect.
14:10	<Stephen Belanger>	Also, I'd really appreciate if people reconsidered much of what James M Snell was was saying in https://github.com/nodejs/node/issues/46262. As far as people that understand the issues of context flow, he's one of very few others I'd trust to understand this stuff, having done a bunch of work on the Cloudflare equivalent of AsyncLocalStorage.