From 37df7415141208a7365759160e69bf11d4cccfa0 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 7 Jul 2017 21:27:33 -0700 Subject: [PATCH] mw.loader: Skip modules in load() with unknown dependencies We already skip unknown modules at the top-level, but dependencies still cause a run-time exception from sortDependencies, resulting in the entire queue not being loaded. Bug: T36853 Change-Id: If8ff31b530dfbd8823c47ffd827fcdba807c05b3 --- resources/src/mediawiki/mediawiki.js | 55 +++++++++++++++---- .../mediawiki/mediawiki.loader.test.js | 48 +++++++++++++++- 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index d172a39ccb..7a835a805b 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -754,6 +754,7 @@ * is used) * - load-callback: exception thrown by user callback * - module-execute: exception thrown by module code + * - resolve: failed to sort dependencies for a module in mw.loader.load * - store-eval: could not evaluate module code cached in localStorage * - store-localstorage-init: localStorage or JSON parse error in mw.loader.store.init * - store-localstorage-json: JSON conversion error in mw.loader.store.set @@ -1169,6 +1170,33 @@ return resolved; } + /** + * Like #resolve(), except it will silently ignore modules that + * are missing or have missing dependencies. + * + * @private + * @param {string[]} modules Array of string module names + * @return {Array} List of dependencies. + */ + function resolveStubbornly( modules ) { + var i, saved, resolved = []; + for ( i = 0; i < modules.length; i++ ) { + saved = resolved.slice(); + try { + sortDependencies( modules[ i ], resolved ); + } catch ( err ) { + // This module is unknown or has unknown dependencies. + // Undo any incomplete resolutions made and keep going. + resolved = saved; + mw.track( 'resourceloader.exception', { + exception: err, + source: 'resolve' + } ); + } + } + return resolved; + } + /** * Load and execute a script. * @@ -2012,6 +2040,15 @@ /** * Load an external script or one or more modules. * + * This method takes a list of unrelated modules. Use cases: + * + * - A web page will be composed of many different widgets. These widgets independently + * queue their ResourceLoader modules (`OutputPage::addModules()`). If any of them + * have problems, or are no longer known (e.g. cached HTML), the other modules + * should still be loaded. + * - This method is used for preloading, which must not throw. Later code that + * calls #using() will handle the error. + * * @param {string|Array} modules Either the name of a module, array of modules, * or a URL of an external script or style * @param {string} [type='text/javascript'] MIME type to use if calling with a URL of an @@ -2043,25 +2080,21 @@ modules = [ modules ]; } - // Filter out undefined modules, otherwise resolve() will throw - // an exception for trying to load an undefined module. - // Undefined modules are acceptable here in load(), because load() takes - // an array of unrelated modules, whereas the modules passed to - // using() are related and must all be loaded. + // Filter out top-level modules that are unknown or failed to load before. filtered = $.grep( modules, function ( module ) { var state = mw.loader.getState( module ); return state !== null && state !== 'error' && state !== 'missing'; } ); - - if ( filtered.length === 0 ) { - return; - } - // Resolve entire dependency map - filtered = resolve( filtered ); + // Resolve remaining list using the known dependency tree. + // This also filters out modules with unknown dependencies. (T36853) + filtered = resolveStubbornly( filtered ); // If all modules are ready, or if any modules have errors, nothing to be done. if ( allReady( filtered ) || anyFailed( filtered ) ) { return; } + if ( filtered.length === 0 ) { + return; + } // Some modules are not yet ready, add to module load queue. enqueue( filtered, undefined, undefined ); }, diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js index 06ea9bc58f..9dc9e5d661 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js @@ -191,14 +191,30 @@ } ); QUnit.test( '.load() - Error: Circular dependency', function ( assert ) { + var capture = []; mw.loader.register( [ [ 'test.circleA', '0', [ 'test.circleB' ] ], [ 'test.circleB', '0', [ 'test.circleC' ] ], [ 'test.circleC', '0', [ 'test.circleA' ] ] ] ); - assert.throws( function () { - mw.loader.load( 'test.circleC' ); - }, /Circular/, 'Detect circular dependency' ); + this.sandbox.stub( mw, 'track', function ( topic, data ) { + capture.push( { + topic: topic, + error: data.exception && data.exception.message, + source: data.source + } ); + } ); + + mw.loader.load( 'test.circleC' ); + assert.deepEqual( + [ { + topic: 'resourceloader.exception', + error: 'Circular reference detected: test.circleB -> test.circleC', + source: 'resolve' + } ], + capture, + 'Detect circular dependency' + ); } ); QUnit.test( '.using() - Error: Unregistered', function ( assert ) { @@ -219,6 +235,32 @@ mw.loader.load( 'test.using.unreg2' ); } ); + // Regression test for T36853 + QUnit.test( '.load() - Error: Missing dependency', function ( assert ) { + var capture = []; + this.sandbox.stub( mw, 'track', function ( topic, data ) { + capture.push( { + topic: topic, + error: data.exception && data.exception.message, + source: data.source + } ); + } ); + + mw.loader.register( [ + [ 'test.load.missingdep1', '0', [ 'test.load.missingdep2' ] ], + [ 'test.load.missingdep', '0', [ 'test.load.missingdep1' ] ] + ] ); + mw.loader.load( 'test.load.missingdep' ); + assert.deepEqual( + [ { + topic: 'resourceloader.exception', + error: 'Unknown dependency: test.load.missingdep2', + source: 'resolve' + } ], + capture + ); + } ); + QUnit.test( '.implement( styles={ "css": [text, ..] } )', function ( assert ) { var $element = $( '
' ).appendTo( '#qunit-fixture' ); -- 2.20.1