mw.loader: Skip modules in load() with unknown dependencies
authorTimo Tijhof <krinklemail@gmail.com>
Sat, 8 Jul 2017 04:27:33 +0000 (21:27 -0700)
committerKrinkle <krinklemail@gmail.com>
Sat, 8 Jul 2017 22:53:03 +0000 (22:53 +0000)
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
tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js

index d172a39..7a835a8 100644 (file)
                         *     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
                                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.
                         *
                                /**
                                 * 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
                                                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 );
                                },
index 06ea9bc..9dc9e5d 100644 (file)
        } );
 
        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 ) {
                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 = $( '<div class="mw-test-implement-a"></div>' ).appendTo( '#qunit-fixture' );