Asynchronous initialization presents a problem for many node modules. One common pattern for ensuring initialization is only done once is to wrap initialization in a promise that is "thened" on each module operation:

var initialize = new Promise(function(resolve, reject) {
  resolveMetadata(function(err, response) {
    if (err) reject(err);
    resolve(response.metadata);
  });
});

function foo(callback) {
  initialize.then(function(metadata) {
    doOperation(metadata, callback);
  });
}

module.exports = foo;

Because continuation-local-storage propagates context from a promise's creation context into the then function, this will cause each call to foo made from inside a namespace#run to lose context. This is problematic if a modules is trying to handle each incoming request to a web server in a separate namespace and foo is invoked per request.

This small test case demonstrates the issue:

var cls = require('continuation-local-storage');
var ns = cls.createNamespace('foo');

var p = new Promise(function(res, rej) {
  res(5);
});

ns.run(function() {
  ns.set('a', 'b');
  p.then(function() {
    console.log(ns.get('a')); // prints undefined but should print 'b'
  });
});

It would be great if then only ran in p's context if there was a context at the time p was created. To play around with this issue, I tried this async-listener patch:

diff --git a/index.js b/index.js
index e634ac4..51649bd 100644
--- a/index.js
+++ b/index.js
@@ -490,14 +490,14 @@ function wrapPromise() {
       // continuations of the accept or reject call using the __asl_wrapper created above.
       function bind(fn) {
         if (typeof fn !== 'function') return fn;
-        return function (val) {
+        return wrapCallback(function (val) {
           var result = (promise.__asl_wrapper || propagateAslWrapper)(this, fn, val, next);
           if (result.error) {
             throw result.errorVal
           } else {
             return result.returnVal
           }
-        };
+        });
       }
     }
   }

This patch will result in the above code snippet printing 'b' without clobbering context if then is called at point with no active namespace:

var cls = require('continuation-local-storage');
var ns = cls.createNamespace('foo');

var p2;

ns.run(function() {
  ns.set('c', 'd');
  p2 = new Promise(function(res, rej) {
    res(6);
  });
});

ns.run(function() {
  ns.set('c', 'e');
  p2.then(function() {
    console.log(ns.get('c')); // prints 'd' with and without patch
  });
});

While this change (or similar) will not address all issues CLS has with promises, it will reduce the amount of patching APM vendors need to do for this relatively common pattern. What do others think about a change like this? /cc @Qard @watson @ofrobots