othiym23 / async-listener

polyfill version of the 0.11 version of the asyncListener API
https://www.npmjs.org/package/async-listener
BSD 2-Clause "Simplified" License
174 stars 52 forks source link

Promise.all context propagation #59

Open jbellenger opened 8 years ago

jbellenger commented 8 years ago

Async-listener will propagate contexts through Promise.all, which is different behavior from similar functionality in the domain module.

It's not 100% clear to me that this is a bug, though it is unexpected.

Test case:

'use strict';

// CLS
(() => {
  const cls = require('continuation-local-storage');
  const ns = cls.createNamespace('ns');
  const promises = [];

  for (let i=0; i < 3; i++) {
    const p = new Promise((resolve) => {
      ns.run(() => {
        ns.active.i = i;
        resolve(i)
      });
    });

    promises.push(p);
  }

  Promise.all(promises)
    .then(() => {
      console.log('[CLS] Promise.all active ns', ns.active);
    });
})();

// domain
(() => {
  const domain = require('domain');
  const d = domain.createDomain();
  const promises = [];

  for (let i=0; i < 3; i++) {
    const p = new Promise((resolve) => {
      d.run(() => {
        domain.active.i = i;
        resolve(i);
      });
    });

    promises.push(p);
  };

  Promise.all(promises)
    .then(() => {
      console.log('[Domain] Promise.all active domain', d.active);
    });
})();

Output:

$ node -version
v6.2.2

$ node cls-test.js
[CLS] Promise.all active ns { i: 2 }
[Domain] Promise.all active domain undefined
hayes commented 8 years ago

This is by design, although there is a lot of debate as too the correct approach. There is currently an open issue on CLS discussig how promises should propagate contexts. The approach asl took was to propagate from resolution to callback rather than creation to callback. For example, if a promise wraps an api that makes a network request, the callback passed to then would be a child of that request rather than a sibling. This allows consumers of the asl api to trace causality. In the Promise.all case, the thing resolving the promise is the same thing as the last promise to resolve that was passed to Promise.all.