error throw inside promise.then or promise.catch function, can not get the right context

[carlisliu]

Namespace remains active if error is thrown in catch of Promise
any idea why?

[nishch]

Any update on this?

[carlisliu]

Use Promise.reject(error) instead of throw, or clear expired context by using setInterval as a workaround solution.

setInterval(()=>{
    let names = Object.keys(process.namespaces || {});
    for(let n of names) {
        let namespace = process.namespaces[n];
        if (expired(namespace )){
            delete process.namespaces[n];
        }
    }
}, 60 * 1000);
function expired(namespace){
    //check if context is expired, usually by adding time stamp
}

[hayes]

I think this is by design. Any call back that runs as a result of a rejected promise is conceptually a continuation from the point the error was thrown.

If you want to stop the propagation of contexts into an error handler, you'll need to handle that explicitly

[nishch]

@hayes @carlisliu thanks for the response I really appreciate it. I think I did a bad job describing the issue correctly. Let's have a look at the following code:

const cls = require("continuation-local-storage");

const ns = cls.createNamespace("namespace1");

ns.run(() => {
    ns.set("name", "Amazing Name");

    setTimeout(() => {
        Promise.resolve()
            .then(() => {
                for (var i = 0; i < 10; i++) {
                    if (i === 8) {
                        console.log(ns.get("name"));
                        throw new Error("this is an error");
                    }
                }
            })
            .catch(err => {
                console.log("catching the errors");
            });
    });
}, 1000);

setTimeout(() => {
    console.log(ns.get("name"));
}, 2000)

I would be expecting the following output in the console:

Amazing Name
catching the errors
undefined

However what I get back is following:

Amazing Name
catching the errors
Amazing Name

@carlisliu going with your suggestion, if I do use Promise.reject instead of throw it does seem to be working fine, however my concern is what about the unhandled exceptions? I mean in cases where piece of code inside then block throws an exception which I am not wrapping in try/catch? Doesn't that defeat the whole point of having just one catch block at the end of the promise chain?

Regarding the setTimeout to clear the context does not really help much in my case since I am using cls for instrumentation in a server application, in case of multiple parallel requests, it becomes unpredictable.

Please let me know your thoughts, I do not want to follow anti patters and refactor a huge code base with try/catch floating around everywhere

[hayes]

I can try this after work, but I have a theory. I can test it out when I get off work tonight, but the 2 cases I am currious about are removing the catch and/or replacing it with a then (and not throwing) to confirm it's related to the throwing of an error. The other is putting the ns.run into a timeout of 0.

[hayes]

I have not had time to understand why, but this is the change where this behavior started: 1b94097

[hayes]

This happens because wrapCallback was designed to wrap a top level callback which does not trap errors (it expects a throw to fall through to process._fatalException). Since the error is being thrown in a promise callback, the after handlers do not get run (because of the throw) and the error handlers are also not run (because of the promise handles the error before it get to the fatal exception handler).

@matthewloring I think we will need to either run the error or after handlers in the case of an error, but I'm not sure which is correct, do you have more insight into the use case behind that change?

[tefnout]

Hi all !

Any update on this ?
On my project I get back to v0.6.4 but I guess I'm missing some important fixes doing this.

[rochdev]

If I understand the commit correctly, I think it can be done in other ways external to this library. In any case, that commit basically completely broke the library. I would therefore probably simply revert the change.

For example:

const cls = require('continuation-local-storage')
const ns = cls.createNamespace('test')

ns.run(() => {
  let promise

  ns.set('foo', 'bar')

  ns.run(() => {
    ns.set('foo', 'baz')

    promise = new Promise((resolve, reject) => {
      resolve()
    })
  })

  promise
    .then(() => {
      console.log('Value in resolution context:', ns.get('foo'))
    })

  bind(promise)
    .then(() => {
      console.log('Value in then context:', ns.get('foo'))
    })
})

function bind (promise) {
  const deferred = defer()

  return promise
    .then(function () {
      deferred.resolve.apply(deferred, arguments)
      return deferred.promise
    })
    .catch(function () {
      deferred.reject.apply(deferred, arguments)
      return deferred.promise
    })
}

function defer () {
  const deferred = {}

  deferred.promise = new Promise((resolve, reject) => {
    deferred.resolve = ns.bind(resolve)
    deferred.reject = ns.bind(reject)
  })

  return deferred
}

@hayes Thoughts?