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

Error handling with domains doesn't works well #1467

Closed
al6x opened this issue Jan 11, 2013 · 2 comments
Closed

Error handling with domains doesn't works well #1467

al6x opened this issue Jan 11, 2013 · 2 comments

Comments

@al6x
Copy link

al6x commented Jan 11, 2013

try/catch statement in the next interferes with error handling via domains.

Try this example, one error will be catch another will be not. It's not a critical issue, there are ways to workaround it, but would be nice if it just worked out of the box.

var Domain  = require('domain')
var express = require('express')

var app = express.createServer()

app.use(function(req, res, next){
  var domain = Domain.create()
  domain.on('error', function(err){
    res.send("Catched " + err.message)
  })
  domain.run(next)  
})

app.get('/failed-to-catch', function(req, res){
  throw new Error('hello there!')
})

app.get('/catched', function(req, res){
  process.nextTick(function(){throw new Error('hello there!')})
})

app.listen(3000)
@tj
Copy link
Member

tj commented Jan 13, 2013

definitely dont want to remove existing error handling just to let things bubble up for domains, at least until they're proven, closing for now

@emostar
Copy link

emostar commented May 31, 2013

I think express can support domains more naturally by having the router call the functions inside a process.nextTick.

This is a one line change like this:

    if (fn.length < 4) return process.nextTick(function() {fn(req, res, callbacks)});

Would there be any undesired side effects from doing that?

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