Erroring in event handlers

[frankiebee]

event emitters fired in catches will run the listeners in the context of the catch. If it fails, however unrelated to the the function that fires the event it will stop the execution of the catch. this can cause serious bugs such as issue #3881 to not be easy to identify. And the error messages they produce can be unrelated to the reason the method failed in the first place. and in the case of txController#approveTransaction a failed to post to sentry is neither a useful error for the dapp nor the reason why the method failed. One short term solution is to wrap event emitting methods in a setTimeout so the listeners are executing outside the context of the catch. But the better long term solution is to not have events emitted in catch this of course will require a rewrite of the architecture of txController.

[kumavis]

we can wrap our emits in try-catch's
this is a design flaw in eventemitters
i think dtar said we should use streams instead of event emitters (just push all events through streams, filter for what event we care about)
we can solve niceties likeonce with some abstractions

[frankiebee]

if we run it in a try catch what am i going to do with the error? i can log it but if the error is related to posting to sentry and i error in the catch cause of logging we run into the same problem the error is thrown in the catch throwing in the catch above it silently killing approve transaction

[kumavis]

yeah we can log.error(err) and drop it then continue normally

[bdresser]

closing since #4131 was merged