AdguardTeam / PopupBlocker

Popup blocking userscript
GNU Lesser General Public License v3.0
337 stars 31 forks source link

Handle exceptions in `wrapMethod` and `wrapAccessor` implementation #91

Closed theseanl closed 4 years ago

theseanl commented 6 years ago

Currently, if an uncaught error occurs in an ApplyHandler function passed to wrapMethod or wrapAccessor, it may break a third-party code that invoked a wrapped function. Furthermore, if such an error occurs before the original function -- usually denoted by orig variable in implementations -- is called, the original function will end up not being called.

The problem is that our code is called from third-party code and such code may not have exception handling (rightfully so in many cases).

What we should do is to make wrapMethod implementation explicitly handle errors. To do so, we need to change the interface of ApplyHandler. A tentative plan is to make it accepts a parameter origCaller, which is supposed to be called only once during a control flow of ApplyHandler. wrapMethod implementation should wrap invocation of ApplyHandlers with try-catch block, and if origCaller is not called, it should call the default apply handler and log errors so that we can fix in future versions.

Additionally, in dev version, if origCaller is called more than once, we should throw an error in the wrapped function.

theseanl commented 6 years ago

Implemented in https://github.com/AdguardTeam/PopupBlocker/commit/d93fcaa5ce30fe401674fc5a83e8ea56640579d6