Error handling: try to stringify non-error throw if it is an object #1113
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1113 +/- ##
==========================================
+ Coverage 99.73% 99.73% +<.01%
==========================================
Files 5 5
Lines 371 372 +1
==========================================
+ Hits 370 371 +1
Misses 1 1
Continue to review full report at Codecov.
|
Alexsey
changed the title
lib/context.js
Outdated
| if (!(err instanceof Error)) { | ||
| try { | ||
| if (Object.prototype.toString.call(err) === '[object Object]') { | ||
| err = JSON.stringify(err); |
There was a problem hiding this comment.
err must convert to Error instance, BTW can we use new Error(util.format('non-error thrown: %j', err) here?
There was a problem hiding this comment.
What is the reason to convert it to an error if on the next line it would be converted to a string with ${err}? And generally there is no sense in the conversion because stack trace is already lost - new Error would have a stack trace to Koa internals which is not helpful :( Although usage of util.format may be a good idea - I'll try to play with it to make a code nicer
There was a problem hiding this comment.
it will emit in app here, we must keep err to be an instance of Error so it won't break anyone's code.
There was a problem hiding this comment.
Hm. But it would already always become an error two lines lower
There was a problem hiding this comment.
You where right about util.format - it really covers all this cases without ifs and additional code! Consider a new version now
Alexsey
force-pushed
the
nonErrorHandling
branch
2 times, most recently
from
04bc7e6
to
786ea3d
Compare
Alexsey
force-pushed
the
nonErrorHandling
branch
from
786ea3d
to
5c1c1c3
Compare
|
There is still one case that |
|
this is enough, throw a literal is an edge case, and throw a symbol is meaningless. |
In case when object were thrown error message is really non helpful - you just see
non-error thrown: [object Object]and stack trace is leads to Koa source. If someone throws an object - it is his fault that no stack trace would be provided, but seen the object itself is already a good start. A specially when it was not your code but some of the modules that you usedWith this PR thrown objects would be converted to JSON