Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespace remains active if error is thrown in catch of Promise #119

Open
apalsapure opened this issue Jun 1, 2017 · 2 comments
Open

Namespace remains active if error is thrown in catch of Promise #119

apalsapure opened this issue Jun 1, 2017 · 2 comments

Comments

@apalsapure
Copy link

I'm using this module in my express app. As you suggested I have used it as a request context and stopped setting properties on request and response.
But I ran into a problem. When an error is thrown inside Promise, active context(namespace) never exits. All the calls made after this; share same context.

Following is a small code snippet which shows the issue.

var express = require('express')
var app = express();
var getNamespace = require('continuation-local-storage').getNamespace;
var nameSpaceName = 'sample';
var sampleKey = 'sampleKey';

var createNamespace = require('continuation-local-storage').createNamespace;
createNamespace(nameSpaceName);

// add middle-ware
app.use(function (req, res, next) {
  var namespace = getNamespace(nameSpaceName);

  // hack for exiting context <=========
  if (namespace.active) {
    console.log(namespace.get(sampleKey)); // prints `sample` instead of `undefined`
    console.log('calling exit');
    namespace.exit(namespace.active);
  }

  namespace.bindEmitter(req);
  namespace.bindEmitter(res);

  namespace.run(function () {
    next();
  });
});

// throws an error in promise
var errorPromise = function () {
  var promise = new Promise((resolve, reject) => {
    reject('str');
  });
  Promise.resolve(promise).then(function (obj) {
  }).catch(function (err) {
    throw new Error(err);
  });
}

// route
app.get('/', function (req, res) {
  // adding sample key in context
  // which should reset in every call
  var namespace = getNamespace(nameSpaceName);
  namespace.set(sampleKey, 'sample');

  // if request query as test, call promise
  if (req.query.test)
    errorPromise();

  res.status(200).send('{"status": 200}');
});
app.listen(3000, function () {
  console.log('Example app listening on port 3000!')
})

Here when query string contains test, call to error promise is made. After this error when next call is made, the earlier context is still active.

Following are the requests made and respective console output

http://localhost:3000/
http://localhost:3000/?test=true
http://localhost:3000/

output:
Example app listening on port 3000!
(node:796) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): Error: str
sample
calling exit

For now before I call namespace.run, if the namespace is active, I explicitly call exit on it. But this is not the right solution, as in parallel calls for few milliseconds the context is shared.

Please let me know if I'm doing something wrong or propose a solution.

Thanks.

@heycalmdown
Copy link

I don't understand why this happens, but here is an ad-hoc for this.

var errorPromise = function () {
  var promise = new Promise((resolve, reject) => {
    reject('str');
  });
  Promise.resolve(promise).then(function (obj) {
  }).catch(function (err) {
    //throw new Error(err);
    return Promise.reject(err);
  });
}

@carlisliu
Copy link

Actually, I think it's a async-listener problem, not continuation-local-storage. In this case, when throw an error in promise.catch method, it trigger 'unhandledRejection' instead of calling process._fatalException function. Missing calling for asyncCatcher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants