fix: Pull Error's name from constructor, not Error itself #372
Conversation
kamilogorek
force-pushed
the
error-name
branch
from
e601244 to
b26650d
Compare
lib/parsers.js
Outdated
| @@ -15,7 +15,7 @@ module.exports.parseText = function parseText(message, kwargs) { | |||
|
|
|||
| module.exports.parseError = function parseError(err, kwargs, cb) { | |||
| utils.parseStack(err, function(frames) { | |||
| var name = err.name + ''; | |||
| var name = err.constructor.name + ''; | |||
There was a problem hiding this comment.
Should it not be:
var name = (err.name || err.constructor.name) + '';That way if someone sets a local name property, that is prioritized over going up the constructor chain?
There was a problem hiding this comment.
If one won't set err.name manually, then it defaults to Error, which will cause the code to never make it through the OR operator. Therefore err.constructor.name will always be skipped in this scenario.
There was a problem hiding this comment.
How about:
var name = {}.hasOwnProperty.call(err, 'name') ?
err.name : err.constructor.name;My interest is in maintaining the behavior where someone has set err.name manually; I'd like this to keep working without them having to keep up to date w/ the changelog.
There was a problem hiding this comment.
Got it. Updated and added additional tests covering this cases.
kamilogorek
force-pushed
the
error-name
branch
2 times, most recently
from
16110a7 to
de24ed9
Compare
kamilogorek
force-pushed
the
error-name
branch
from
de24ed9 to
5f547c4
Compare
| @@ -15,7 +15,8 @@ module.exports.parseText = function parseText(message, kwargs) { | |||
|
|
|||
| module.exports.parseError = function parseError(err, kwargs, cb) { | |||
| utils.parseStack(err, function(frames) { | |||
| var name = err.name + ''; | |||
| var name = | |||
| ({}.hasOwnProperty.call(err, 'name') ? err.name : err.constructor.name) + ''; | |||
There was a problem hiding this comment.
Please use Object.prototype.hasOwnProperty.call() since you don't create a new object first
There was a problem hiding this comment.
Thanks for pointing that out. However, it's such a micro-optimization, which won't make any difference ever (this object will be collected by GC anyway), that I'll just patch it while working on something else and won't create a separate PR for it. We also use {}. notation in other places, and I prefer code consistency over small performance gains :)
Fixes: #350