Right now, if an exception occurs while executing a ResourceLoader module, the behavior is:
- Catch the exception and log it to the console
- Discard the exception
- Set the module's state to 'error'
- For any modules depending on the failed module, set their state to 'error' as well
- For each error handler attached to this module (with mw.loader.using) or one of its dependencies
- Generate a fake exception saying "Module foo failed"
- Throw it
- Catch it
- Pass it to the error handler
- If the error handler throws an exception, catch it and log it
So the error handler doesn't receive the original exception, it receives a fake one that we generate separately for each handler, then throw and then catch again. That seems ridiculous, we should just store the original exception and pass it in. For dependent modules I suppose a fake exception is reasonable, maybe, but for an error handler attached to the module itself (or really to a set of modules including the module itself) surely we should pass the real exception.
What's even weirded is when a ResourceLoader module executes successfully, but a success callback attached to it (with mw.loader.using) throws an exception:
- Module execution succeeds
- Call success callback
- Get exception from success callback
- Catch it
- If there is an error handler, call it and pass the caught exception
- If the error handler also throws an exception, catch it and log it
So the exception from the error handler is caught but never logged. Also, the error handler is invoked after the success handler has already been invoked, which means that if you hook up success/error to a promise's resolve/reject, we'll try to reject a promise that's already been resolved, which is a no-op and doesn't call the .fail() handlers. Meanwhile, the exception is completely buried and untraceable.
mw.loader shouldn't be trying to catch exceptions that occur in success handlers that live in user land and were passed into .using() (this is all ready handlers, we don't use them internally). Exceptions in user land are user land's problem, so we should just let them happen and not catch them. The fact that we call the error callback is even more wrong, because that's for when execution of the module failed, not for when execution of the module succeeded but some random user land code responding to this event ends up crapping itself.
Version: 1.22.0
Severity: normal