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

Use Error.captureStackTrace instead stack modification #17

Closed ikokostya closed 9 years ago

ikokostya commented 9 years ago
[kostya@arch terror]$ node
> throw new Error('msg')
Error: msg
    at repl:1:7
    at REPLServer.self.eval (repl.js:110:21)
    at repl.js:249:20
    at REPLServer.self.eval (repl.js:122:7)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)
    at Interface._ttyWrite (readline.js:760:14)
    at ReadStream.onkeypress (readline.js:99:10)
> var Terror = require('./lib/terror')
undefined
> throw new Terror(500, 'msg')
Terror: msg
    at repl:1:7
    at REPLServer.self.eval (repl.js:110:21)
    at repl.js:249:20
    at REPLServer.self.eval (repl.js:122:7)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)
    at Interface._ttyWrite (readline.js:760:14)
    at ReadStream.onkeypress (readline.js:99:10)
> 
kaero commented 9 years ago

Looks reasonable, redundant c-tor calls in the stacktrace is annoying. I'll review and test it, and merge or discuss solution here.

kaero commented 9 years ago

@golyshevd Error.captureStackTrace(this, this.constructor); looks fine. I think, createError method must be updated too to avoid meaningless call of createError line as following:

SecondError: code two
    at Function.Terror.createError (terror/lib/terror.js:259:17)
    at Object.module.exports.stacktrace (terror/test/stacktrace.js:24:33) 
     ^^^ SecondError.createError was called here ^^^
    at Object.<anonymous> (terror/node_modules/nodeunit/lib/core.js:235:16)
    at terror/node_modules/nodeunit/lib/core.js:235:16
    at Object.exports.runTest (terror/node_modules/nodeunit/lib/core.js:69:9)
    at terror/node_modules/nodeunit/lib/core.js:117:25
    at terror/node_modules/nodeunit/deps/async.js:513:13
    at iterate (terror/node_modules/nodeunit/deps/async.js:123:13)
    at async.forEachSeries (terror/node_modules/nodeunit/deps/async.js:139:9)
    at _concat (terror/node_modules/nodeunit/deps/async.js:512:9)
kaero commented 9 years ago

fixed in 1.0 branch, will be released and intergrated in another modules soon.

https://github.com/nodules/terror/commit/b3f3bdcefd02cef580df88d29cf551e326c98675