CrabDude / trycatch

An asynchronous domain-based exception handler with long stack traces for node.js
MIT License
246 stars 17 forks source link

Weird usage of event emitters loses errors in mongodb #28

Closed Raynos closed 11 years ago

Raynos commented 11 years ago

I'm using trycatch with mongodb. It is not printing long stack traces and only showing a single stack

mongodb re emits errors from one emitter onto another emitter ( https://github.com/mongodb/node-mongodb-native/blob/master/lib/mongodb/connection/base.js#L216 )

I believe it loses the right domain.

Here is a mongodb example that doesnt work with long stack traces

var mongo = require("continuable-mongo")
var Users = mongo("mongodb://localhost:27017/db").collection("users")

trycatch(function () {
    Users.findOne({ email: "fake "}, function (err) {
        throw { "foo": 42 }
    })
}, logError)

function logError(err) {
    console.error("ERROR", err.message)
    console.error(err.stack)
}

This prints

ERROR [object Object]
Error: [object Object]
    at normalizeError (/home/raynos/Documents/colingo-one-on-one/node_modules/trycatch/lib/formatError.js:33:11)
    at formatError (/home/raynos/Documents/colingo-one-on-one/node_modules/trycatch/lib/formatError.js:17:9)
    at Domain.EventEmitter.emit (events.js:95:17)
    at Db.EventEmitter.emit (events.js:70:21)
    at /home/raynos/Documents/colingo-one-on-one/node_modules/continuable-mongo/node_modules/mongodb/lib/mongodb/connection/base.js:217:19
    at process._tickDomainCallback (node.js:459:13)

This is a reproduction of the issue without the mongodb specifics

trycatch(function () {
    setTimeout(function () {
        var EventEmitter = require("events").EventEmitter
        var firstEmitter = new EventEmitter()

        process.nextTick(function () {
            var emitter = new EventEmitter()

            emitter.once("error", function (err) {
                process.nextTick(function () {
                    firstEmitter.emit("error", err)
                })
            })

            emitter.emit("error", { foo: 42 })
        })
    }, 100)
}, logError)

function logError(err) {
    console.error("ERROR", err.message)
    console.error(err.stack)
}

Here is the stack trace (not long)

ERROR [object Object]
Error: [object Object]
    at normalizeError (/home/raynos/Documents/colingo-one-on-one/node_modules/trycatch/lib/formatError.js:33:11)
    at formatError (/home/raynos/Documents/colingo-one-on-one/node_modules/trycatch/lib/formatError.js:17:9)
    at Domain.EventEmitter.emit (events.js:95:17)
    at EventEmitter.emit (events.js:70:21)
    at /home/raynos/Documents/colingo-one-on-one/bin/check-personal-teacher.js:69:34
    at process._tickDomainCallback (node.js:459:13)
raynos at raynos-Latitude-E5530-non-vPro  ~/Documents/colingo-one-on

Is this an issue with trycatch or with mongodb ?

Raynos commented 11 years ago

I tried fixing the reproduction code to do domains "properly"

setTimeout(function () {
        var EventEmitter = require("events").EventEmitter
        var firstEmitter = new EventEmitter()

        process.nextTick(function () {
            var emitter = new EventEmitter()

            var correctDomain = emitter.domain

            emitter.once("error", function (err) {
                process.nextTick(function () {
                    var oldDomain = firstEmitter.domain
                    correctDomain.add(firstEmitter)
                    firstEmitter.emit("error", err)
                    oldDomain.add(firstEmitter)
                })
            })

            emitter.emit("error", { foo: 42 })
        })
    }, 100)

Putting the firstEmitter in the "correct" domain does not seem to help.

Raynos commented 11 years ago

Now that I think about it for a moment I'm not actually sure why the re emit on different event emitter case breaks. This might be a bug in trycatch

CrabDude commented 11 years ago

Fixed: https://github.com/CrabDude/trycatch/commit/426ca705bf4a22658443ababb6443c647b5af1f4