suggestion for verbose error reporting as per #721 #722
Conversation
|
fixes #721 |
| } catch(e){ | ||
| hasException = true; | ||
| handleExceptionInDerivation(derivation, e); | ||
| throw e; |
There was a problem hiding this comment.
The problem with this change is this changes the stack trace of the exception to this line. MobX had something similar in the past, but since the original stack is lost with a catch / error, people will blame MobX for the exception, although it originated in userland, which results in a plethorea of issues in the MobX issue tracker. Sadly this cannot be fixed really nicely in Javascript as far as I know 😢
There was a problem hiding this comment.
The stack trace is generated when new Error is called, so rethrowing the same error object shouldn't change the stack (unless handleExceptionInDerivation mutates the error object)
There was a problem hiding this comment.
what @spion said ...
what do you think about a test to assert the original stack trace?
|
Merged into, and closed in favor of #736. Thanks for stirring up the discussion! |
I didn't find the right place to add tests for the new field in spy error message, would love a point to the right direction :)
time report in
npm run perfseems the same with +-10% range of error , also this change should only slightly affect behavior of systems already in error mode (as the catch blocks were added to already non optimizable functions)