https://bugzilla.wikimedia.org/show_bug.cgi?id=54618
Web browser: ---
Bug ID: 54618
Summary: Make exception handling in mw.loader.using sane
Product: MediaWiki
Version: unspecified
Hardware: All
OS: All
Status: NEW
Severity: normal
Priority: Unprioritized
Component: ResourceLoader
Assignee: wikibugs-l@lists.wikimedia.org
Reporter: roan.katt...@gmail.com
CC: krinklem...@gmail.com, roan.katt...@gmail.com,
tpars...@wikimedia.org
Classification: Unclassified
Mobile Platform: ---
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.
--
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
