Last modified: 2013-10-25 19:58:43 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T57989, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 55989 - ResourceLoader: mw.loader.using shouldn't silently catch exceptions from userland ready() callback
ResourceLoader: mw.loader.using shouldn't silently catch exceptions from user...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
1.19.0
All All
: Normal normal (vote)
: 1.22.0 release
Assigned To: Krinkle
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-22 09:38 UTC by Niklas Laxström
Modified: 2013-10-25 19:58 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Niklas Laxström 2013-10-22 09:38:48 UTC
Nothing is logged to console if error happens inside mw.loader.using callback function. MAkes it hard to debug when code just seems to not execute.
Comment 1 Bartosz Dziewoński 2013-10-22 17:17:55 UTC
Here's the relavant part from the handlePending() function in mediawiki.js. `job.ready` is the ready callback passed to mw.loader.using().

  try {
    if ( hasErrors ) {
      throw new Error( 'Module ' + module + ' failed.');
    } else {
      if ( $.isFunction( job.ready ) ) {
        job.ready();
      }
    }
  } catch ( e ) {
    if ( $.isFunction( job.error ) ) {
      try {
        job.error( e, [module] );
      } catch ( ex ) {
        // A user-defined operation raised an exception. Swallow to protect
        // our state machine!
        log( 'Exception thrown by job.error()', ex );
      }
    }
  }

So, any errors raised by the 'ready' callback will be passed to the 'error' callback – but if it's not defined, they will be silently swallowed. They should probably be passed through to log() as well, which will dump the error and the traceback to the browser console.
Comment 2 Krinkle 2013-10-25 19:14:32 UTC
When a module executes and throws an error, this is afaik always logged to the console (even when not in debug mode).

When a callback of your own making throws an error, this is usually not caught because its none of our business.

Inside `handlePending` however, there is one case where we swallow it to protect the state machine. In that case it should indeed log it to the console, it seems it only does that for the error() callback, not the ready() callback.
Comment 3 Gerrit Notification Bot 2013-10-25 19:15:12 UTC
Change 91981 had a related patch set uploaded by Krinkle:
mw.loader: Always log exceptions caught from userland callbacks

https://gerrit.wikimedia.org/r/91981
Comment 4 Gerrit Notification Bot 2013-10-25 19:38:17 UTC
Change 91981 merged by jenkins-bot:
mw.loader: Always log exceptions caught from userland callbacks

https://gerrit.wikimedia.org/r/91981
Comment 5 Gerrit Notification Bot 2013-10-25 19:48:49 UTC
Change 91987 had a related patch set uploaded by Bartosz Dziewoński:
mw.loader: Always log exceptions caught from userland callbacks

https://gerrit.wikimedia.org/r/91987
Comment 6 Bartosz Dziewoński 2013-10-25 19:50:10 UTC
Fixed by Krinkle, merged and backported to 1.22 per comment 2 (milestone=1.22).
Comment 7 Gerrit Notification Bot 2013-10-25 19:58:43 UTC
Change 91987 merged by jenkins-bot:
mw.loader: Always log exceptions caught from userland callbacks

https://gerrit.wikimedia.org/r/91987

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links