From: Timo Tijhof <krinklemail@gmail.com>
Date: Tue, 8 Oct 2013 21:47:42 +0000 (+0200)
Subject: resourceloader: Don't catch LESS error in ResourceLoaderFileModule
X-Git-Tag: 1.31.0-rc.0~18516
X-Git-Url: http://git.cyclocoop.org/%22.%24image2.%22?a=commitdiff_plain;h=cdc8b9e01113fa78dc211c278a06ff08a4b5bb5a;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Don't catch LESS error in ResourceLoaderFileModule

Instead let it throw up to ResourceLoader::makeModuleResponse.. By using
the central logic for exception handling the parse error becomes more
discoverable and obvious.

* The module is marked as in "error" state (currently it would still serve
  the result of file A and C, even if file B has a parse error).
  This matches the behaviour of the javascript checker we have.
* The exception comment is produced on top of the load.php response
  (where users expect it) instead of buried somewhere between the
  different modules and the different files within the modules.
* The exception comment will not be stripped out by CSSMin.
* DRY :)

Without this, debugging less parse errors is harder than it should be.

Bug: 55442
Change-Id: I6510b43f02cf82568b7fd2421209a8e636d5e115
---

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index c9c7e4edc5..9ed181ed77 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -722,21 +722,17 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 	/**
 	 * Generate a cache key for a LESS file.
 	 *
-	 * The cache key varies on the file name, the names and values of global
-	 * LESS variables, and the value of $wgShowExceptionDetails. Varying on
-	 * $wgShowExceptionDetails ensures the CSS comment indicating compilation
-	 * failure shows the right level of detail.
+	 * The cache key varies on the file name and the names and values of global
+	 * LESS variables.
 	 *
 	 * @since 1.22
 	 * @param string $fileName File name of root LESS file.
 	 * @return string: Cache key
 	 */
 	protected static function getLESSCacheKey( $fileName ) {
-		global $wgShowExceptionDetails;
-
 		$vars = json_encode( ResourceLoader::getLESSVars() );
 		$hash = md5( $fileName . $vars );
-		return wfMemcKey( 'resourceloader', 'less', (string)$wgShowExceptionDetails, $hash );
+		return wfMemcKey( 'resourceloader', 'less', $hash );
 	}
 
 	/**
@@ -753,8 +749,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 	 * @return string: CSS source
 	 */
 	protected function compileLESSFile( $fileName ) {
-		global $wgShowExceptionDetails;
-
 		$key = self::getLESSCacheKey( $fileName );
 		$cache = wfGetCache( CACHE_ANYTHING );
 
@@ -767,34 +761,16 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 		}
 
 		$compiler = ResourceLoader::getLessCompiler();
-		$expire = 0;
-		try {
-			$result = $compiler->cachedCompile( $source );
-			if ( !is_array( $result ) ) {
-				throw new Exception( 'LESS compiler result has type ' . gettype( $result ) . '; array expected.' );
-			}
-		} catch ( Exception $e ) {
-			// The exception might have been caused by an imported file rather
-			// than the root node. But we don't know which files were imported,
-			// because compilation failed; we thus cannot rely on file mtime to
-			// know when to reattempt compilation. Expire in 5 mins. instead.
-			$expire = 300;
-			wfDebugLog( 'resourceloader', __METHOD__ . ": $e" );
-			$result = array();
-			$result['root'] = $fileName;
-
-			if ( $wgShowExceptionDetails ) {
-				$result['compiled'] = ResourceLoader::makeComment( 'LESS error: ' . $e->getMessage() );
-			} else {
-				$result['compiled'] = ResourceLoader::makeComment( 'LESS stylesheet compilation failed. ' .
-					'Set "$wgShowExceptionDetails = true;" to show detailed debugging information.' );
-			}
+		$result = null;
 
-			$result['files'] = array( $fileName => self::safeFilemtime( $fileName ) );
-			$result['updated'] = time();
+		$result = $compiler->cachedCompile( $source );
+
+		if ( !is_array( $result ) ) {
+			throw new MWException( 'LESS compiler result has type ' . gettype( $result ) . '; array expected.' );
 		}
+
 		$this->localFileRefs += array_keys( $result['files'] );
-		$cache->set( $key, $result, $expire );
+		$cache->set( $key, $result );
 		return $result['compiled'];
 	}
 }