Page MenuHomePhabricator

ResourceLoader: mw.loader.using shouldn't silently catch exceptions from userland ready() callback
Closed, ResolvedPublic

Description

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.


Version: 1.19.0
Severity: normal

Details

Reference
bz55989

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:22 AM
bzimport set Reference to bz55989.

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.

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.

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

Change 91981 merged by jenkins-bot:
mw.loader: Always log exceptions caught from userland callbacks

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

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

Fixed by Krinkle, merged and backported to 1.22 per comment 2 (milestone=1.22).

Change 91987 merged by jenkins-bot:
mw.loader: Always log exceptions caught from userland callbacks

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