How to deal with ambgious contexts for promise then callbacks

[ofrobots]

Forking discussion from: #141 (comment)

[ofrobots]

/cc @mike-kaufman @Qard @watson

[mike-kaufman]

@ofrobots said here in reference to this async context visualization

@mike-kaufman that model doesn't match my intuition. It also doesn't match all the potential mental models developers can have, as listed in othiym23/node-continuation-local-storage#64.

My intuition is that Execute 16 is linked by Execute 13, it is caused by Link 9. Or, colloquially, the causal parent of the then callback in the second request should be the lazyPromise.

/cc @mrkmarron .

Mark & I discussed this yesterday. In DLS paper, the claim was (somewhere in there, Mark can point at exact location?) that if you chain onto an already resolved promise, then it's "causal context" is the same as it's "linking context", which is what the graph shows. I'll let @mrkmarron chime in on details of why this is.

I think what you're getting at is you want to know the context that resolved the promise (effectively, the ability to traverse contexts of the promise chain). I think to achieve this, in the DAG you would have Cause 15's causal edge point to Execute 12 (the execution of the prior "then" in the promise chain).

I agree with this. @mrkmarron?

[mrkmarron]

Hi Mike, yes I agree with your comments. The relevant part in the paper is Figure 6 (E-Then-Resolved rule) which differs from then (E-Then-Unresolved rule) by binding both the link and causal context. Thus, attaching a then to an already resolved context will give the causal context at the location of the then instead of where the promise originally resolved. We felt this was:

1. A more honest representation of what was happened in the asynchronous execution and matched our intuitive view of the causal location is the last of the JS code neeed to commit an async invocation to execution.
2. It also seemed to better match intuition for cases like the one you have where, for the initial executions, having the promise resolve as part of the async-chain is desired while later executions on the resolved result should not have this link.

[watson]

In async-listener we ended up monkey patching the callback given to Promise.prototype.then so that we can emit async events around promise then calls

[ofrobots]

@watson, note that the fix you pointed to doesn't solve the general case. It only solves the context loss for the case where the promise gets initialized in a no-context environment. The example I posted above does not work (correctly) with the current implementation of async-listener / continuation-local-storage.

@mike-kaufman, @mrkmarron I'll go over the paper in more detail and respond back if I can or cannot reconcile my mental model. At the moment I am trying to reflect on the semantics that are actually offered by async_hooks (and predecessors like async-listener). I think we need to convince ourselves that the model offered by the paper can be implemented through the semantics offered by async_hooks.

[ofrobots]

To provide a more concrete example for the no-context promise example case, consider:

const app = express();

let authTokenPromise = fetch('some url');
app.get('/', (req, res) => {
  authTokenPromise.then(() => {
    // business logic.
    // What is my context here?
  });
});

async-listener works correctly for this case by virtue of the fix you pointed to.

[mike-kaufman]

@ofrobots - FYI there's code here that makes a half-hearted attempt to generate the events of the proposed model from async-hooks APIs. The causal linking is the missing piece at the moment.

[gireeshpunathil]

should this remain open? [ I am trying to chase dormant issues to closure ]

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.