nodules / terror

Base error class which will help you organize errors, generate informative logs and as result grep logs more effectively
MIT License
25 stars 7 forks source link

Log all nested errors #18

Closed kaero closed 9 years ago

kaero commented 9 years ago

For now, Terror put in the message only message of callee instance and its originalError, but not all nested originalErrors. It's can be confusing for some cases, especially for asker module, if user has set maxRetries option, it can get message of RETRIES_LIMIT_EXCEEDED and HTTP_CLIENT_REQUEST_ERROR, but not source of HTTP_CLIENT_REQUEST_ERROR which is only significant.

kaero commented 9 years ago

\cc @dfilatov

kaero commented 9 years ago

fixed in branch 1.0-dev https://github.com/nodules/terror/commit/3734fa09600c30d39f363b544db2a6ca6a8c6ca1

kaero commented 9 years ago

test case:

var Terror = require('./'),
    MyError = Terror.create('MyError', { FAULT: 'failed, because %reason%' }),
    OtherError = Terror.create('OtherError', { FUCKUP: '%object% went wrong!' });

function simple() {
    OtherError.createError(OtherError.CODES.FUCKUP, { object: 'something' }).log();
}

function nestedError() {
    MyError
        .createError(MyError.CODES.FAULT, new Error('stupid detected'))
        .bind({ reason: 'i am stupid' })
        .log();
}

function multiNested() {
    var err = OtherError
        .createError(OtherError.CODES.FUCKUP, new Error('nothing happens'))
        .bind({ object: 'nothing' });

    MyError
        .createError(MyError.CODES.FAULT, err)
        .bind({ reason: 'i am stupid' })
        .log();
}

simple();
nestedError();
multiNested();

output:

node ./tst
ERROR FUCKUP OtherError: something went wrong!
>>>>>     at simple (/Users/kaero/work/nodules/terror/tst.js:6:16)
>>>>>     at Object.<anonymous> (/Users/kaero/work/nodules/terror/tst.js:27:1)
>>>>>     at Module._compile (module.js:456:26)
>>>>>     at Object.Module._extensions..js (module.js:474:10)
>>>>>     at Module.load (module.js:356:32)
>>>>>     at Function.Module._load (module.js:312:12)
>>>>>     at Function.Module.runMain (module.js:497:10)
>>>>>     at startup (node.js:119:16)
>>>>>     at node.js:906:3
ERROR FAULT MyError: failed, because i am stupid (Error: stupid detected)
>>>>>     at nestedError (/Users/kaero/work/nodules/terror/tst.js:11:10)
>>>>>     at Object.<anonymous> (/Users/kaero/work/nodules/terror/tst.js:28:1)
>>>>>     at Module._compile (module.js:456:26)
>>>>>     at Object.Module._extensions..js (module.js:474:10)
>>>>>     at Module.load (module.js:356:32)
>>>>>     at Function.Module._load (module.js:312:12)
>>>>>     at Function.Module.runMain (module.js:497:10)
>>>>>     at startup (node.js:119:16)
>>>>>     at node.js:906:3
ERROR FAULT MyError: failed, because i am stupid (OtherError: nothing went wrong! (Error: nothing happens))
>>>>>     at multiNested (/Users/kaero/work/nodules/terror/tst.js:22:10)
>>>>>     at Object.<anonymous> (/Users/kaero/work/nodules/terror/tst.js:29:1)
>>>>>     at Module._compile (module.js:456:26)
>>>>>     at Object.Module._extensions..js (module.js:474:10)
>>>>>     at Module.load (module.js:356:32)
>>>>>     at Function.Module._load (module.js:312:12)
>>>>>     at Function.Module.runMain (module.js:497:10)
>>>>>     at startup (node.js:119:16)
>>>>>     at node.js:906:3
dfilatov commented 9 years ago

Why not use message from original error as main message? It's very annoing to check .originalError everywhere.

kaero commented 9 years ago

Why you may need to check original error everywhere? Hiding source error is a feature of terror to isolate implementation errors on the level of module API, so one error in the module can incapsulate some different errors from laying down implementation. Client code, in the most cases, which i saw, cann't fix these error in runtime and can only log them. For logging purposes message of the parent issues now includes messages of all nested errors, as you can see in the example above.

dfilatov commented 9 years ago

I should use specific log() function in that case. Let's imagine you have a chain of promises and you want to log all of errors at the end of the chain. You may have not only asker errors in your chain. You should find out what type of error you have to properly log it. Something like this:

.fail(function(e) {
    if(e instanceof TError) {
        logger.log(TError.log());
    }
    else {
        logger.log(e.message);
    }
});
kaero commented 9 years ago

Since terror 1.0 e.message contains error message, which includes nested error messages. Same for e.stack, which contains an error message on the first line.

Asker dependency will be updated as soon as terror 1.0 will be released, now it's on review.

dfilatov commented 9 years ago

Ok, I'm looking forward to it )

kaero commented 9 years ago

Example, without using Terror#log():

var Terror = require('./'),
    MyError = Terror.create('MyError', { FAULT: 'failed, because %reason%' }),
    OtherError = Terror.create('OtherError', { FUCKUP: '%object% went wrong!' });

function simple() {
    var e = OtherError.createError(OtherError.CODES.FUCKUP, { object: 'something' });
    console.error(e.toString());
}

function nestedError() {
    var e = MyError
        .createError(MyError.CODES.FAULT, new Error('stupid detected'))
        .bind({ reason: 'i am stupid' });
    console.error(e.toString());
}

function multiNested() {
    var err = OtherError
        .createError(OtherError.CODES.FUCKUP, new Error('nothing happens'))
        .bind({ object: 'nothing' });

    var e = MyError
        .createError(MyError.CODES.FAULT, err)
        .bind({ reason: 'i am stupid' });
    console.error(e.toString());
}

simple();
nestedError();
multiNested();

output:

$ node ex.js
OtherError: something went wrong!
MyError: failed, because i am stupid (Error: stupid detected)
MyError: failed, because i am stupid (OtherError: nothing went wrong! (Error: nothing happens))
kaero commented 9 years ago

Ok, I'm looking forward to it )

Fine, thanks for issue report! :)