jenkins-bot has submitted this change and it was merged.
Change subject: resourceloader: Use state "error" instead of "missing" in case
of exceptions
......................................................................
resourceloader: Use state "error" instead of "missing" in case of exceptions
Changes:
* If we catch an exception while making the response for a module, set its
client state to "error" instead of "missing".
State "missing" should only be used if the module could not be
found in the registry.
This matches the behaviour of the client.
Clean up:
* Merge the two mw.loader.state calls into one.
* Add @throws documentation for compileLESSFile (follows-up cdc8b9e).
* Don't use 3 different ways to assert an array being empty,
using 1 and sticking to it.
- !$arr
- $arr === array()
- count( $arr )
Change-Id: I54c3de6d836702ffbe3044bc58a38e83e758bc33
---
M includes/resourceloader/ResourceLoader.php
M includes/resourceloader/ResourceLoaderFileModule.php
2 files changed, 30 insertions(+), 16 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index 6380efc..91acd11 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -68,7 +68,8 @@
*/
public function preloadModuleInfo( array $modules,
ResourceLoaderContext $context ) {
if ( !count( $modules ) ) {
- return; // or else Database*::select() will explode,
plus it's cheaper!
+ // Or else Database*::select() will explode, plus it's
cheaper!
+ return;
}
$dbr = wfGetDB( DB_SLAVE );
$skin = $context->getSkin();
@@ -451,7 +452,7 @@
wfProfileIn( __METHOD__ );
$errors = '';
- // Split requested modules into two groups, modules and missing
+ // Find out which modules are missing and instantiate the others
$modules = array();
$missing = array();
foreach ( $context->getModules() as $name ) {
@@ -527,7 +528,7 @@
}
// Save response to file cache unless there are errors
- if ( isset( $fileCache ) && !$errors && !$missing ) {
+ if ( isset( $fileCache ) && !$errors && !count( $missing ) ) {
// Cache single modules...and other requests if there
are enough hits
if ( ResourceFileCache::useFileCache( $context ) ) {
if ( $fileCache->isCacheWorthy() ) {
@@ -704,21 +705,24 @@
/**
* Generates code for a response
*
- * @param $context ResourceLoaderContext: Context in which to generate
a response
+ * @param $context ResourceLoaderContext Context in which to generate a
response
* @param array $modules List of module objects keyed by module name
- * @param array $missing List of unavailable modules (optional)
- * @return String: Response data
+ * @param array $missing List of requested module names that are
unregistered (optional)
+ * @return string Response data
*/
public function makeModuleResponse( ResourceLoaderContext $context,
- array $modules, $missing = array()
+ array $modules, array $missing = array()
) {
$out = '';
$exceptions = '';
- if ( $modules === array() && $missing === array() ) {
+ $states = array();
+
+ if ( !count( $modules ) && !count( $missing ) ) {
return '/* No modules requested. Max made me put this
here */';
}
wfProfileIn( __METHOD__ );
+
// Pre-fetch blobs
if ( $context->shouldIncludeMessages() ) {
try {
@@ -732,6 +736,11 @@
}
} else {
$blobs = array();
+ }
+
+
+ foreach ( $missing as $name ) {
+ $states[$name] = 'missing';
}
// Generate output
@@ -838,8 +847,8 @@
// Add exception to the output as a comment
$exceptions .= self::formatException( $e );
- // Register module as missing
- $missing[] = $name;
+ // Respond to client with error-state instead
of module implementation
+ $states[$name] = 'error';
unset( $modules[$name] );
}
$isRaw |= $module->isRaw();
@@ -848,14 +857,17 @@
// Update module states
if ( $context->shouldIncludeScripts() && !$context->getRaw() &&
!$isRaw ) {
- // Set the state of modules loaded as only scripts to
ready
if ( count( $modules ) && $context->getOnly() ===
'scripts' ) {
- $out .= self::makeLoaderStateScript(
- array_fill_keys( array_keys( $modules
), 'ready' ) );
+ // Set the state of modules loaded as only
scripts to ready as
+ // they don't have an mw.loader.implement
wrapper that sets the state
+ foreach ( $modules as $name => $module ) {
+ $states[$name] = 'ready';
+ }
}
- // Set the state of modules which were requested but
unavailable as missing
- if ( is_array( $missing ) && count( $missing ) ) {
- $out .= self::makeLoaderStateScript(
array_fill_keys( $missing, 'missing' ) );
+
+ // Set the state of modules we didn't respond to with
mw.loader.implement
+ if ( count( $states ) ) {
+ $out .= self::makeLoaderStateScript( $states );
}
}
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php
b/includes/resourceloader/ResourceLoaderFileModule.php
index 9ed181e..7b85001 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -745,6 +745,8 @@
* recompiles as necessary.
*
* @since 1.22
+ * @throws Exception If Less encounters a parse error
+ * @throws MWException If Less compilation returns unexpection result
* @param string $fileName File path of LESS source
* @return string: CSS source
*/
--
To view, visit https://gerrit.wikimedia.org/r/90320
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I54c3de6d836702ffbe3044bc58a38e83e758bc33
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
