From 6e117a95316aacb98e5268d2c1682e16e9fc9019 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 24 Sep 2019 00:47:18 +0100 Subject: [PATCH] resourceloader: Reduce severity of unknown page module warning These are silently skipped and generally no cause for alarm. For example, when enabling a gadget by default and then disabling/deleting it, cached HTML will still have it queued, which is silently skipped and the other queued modules are loaded just fine. The same for when merging module bundles where the destination is already loaded, in such case it is tolerated to let cached ParserOutput objects still queuing it silently skip it. Keeping it existent just to avoid a warning isn't particularly useful and might even obscure problems or give the illusion that it is still working and providing compatibility (e.g. loading the new code), which might not be true. The silent skipping of modules originally did not have any logging or warning attached to it, it was silent from 2010 to April 2019. In 2017, commit 37df741514 adds a console warning when a circular dependency is detected. In April 2019, commit d059dffa2 optimised away this redundant silent skipping of unknown modules, in favour of re-using the code for circular dep detection, which did the same thing already, but with a debug warning. This meant it now showed scary stack traces when all there's only an stale module ref, which are normal in RL and generaly nothing to worry about. This was amplified by Minerva's tracking of client-side errors going beyond just 'global.error' but also listerning to recoverable internal errors from 'resourceloader.exception', which now included these debug warnings. Change-Id: Ie71adbe18e8dbeb661ddb9d7d3d1d0897891d515 --- resources/src/startup/mediawiki.js | 19 +++++++++++++------ .../mediawiki/mediawiki.loader.test.js | 17 +++-------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/resources/src/startup/mediawiki.js b/resources/src/startup/mediawiki.js index 6a6aa3d885..f814d89609 100644 --- a/resources/src/startup/mediawiki.js +++ b/resources/src/startup/mediawiki.js @@ -969,13 +969,20 @@ try { sortDependencies( modules[ i ], resolved ); } catch ( err ) { - // This module is unknown or has unknown dependencies. - // Undo any incomplete resolutions made and keep going. + // This module is not currently known, or has invalid dependencies. + // Most likely due to a cached reference after the module was + // removed, otherwise made redundant, or omitted from the registry + // by the ResourceLoader "target" system. resolved = saved; - mw.trackError( 'resourceloader.exception', { - exception: err, - source: 'resolve' - } ); + mw.log.warn( 'Skipped unresolvable module ' + modules[ i ] ); + if ( modules[ i ] in registry ) { + // If the module was known but had unknown or circular dependencies, + // also track it as an error. + mw.trackError( 'resourceloader.exception', { + exception: err, + source: 'resolve' + } ); + } } } return resolved; diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js index 4c6e7c98f7..59672f4b1c 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js @@ -246,23 +246,12 @@ QUnit.test( '.load() - Error: Unregistered', 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 - } ); + this.sandbox.stub( mw.log, 'warn', function ( str ) { + capture.push( str ); } ); mw.loader.load( 'test.load.unreg' ); - assert.deepEqual( - [ { - topic: 'resourceloader.exception', - error: 'Unknown module: test.load.unreg', - source: 'resolve' - } ], - capture - ); + assert.deepEqual( capture, [ 'Skipped unresolvable module test.load.unreg' ] ); } ); // Regression test for T36853 -- 2.20.1