Log errors in thumb.php

Add new streamFileWithStatus() methods to FileRepo and
MediaTransformOutput that can be used to get more detailed error
information on failure. The historic streamFile() methods become sinple
wrappers to the new methods. Thumb.php is changed to use the
streamFileWithStatus() methods so that failure reasons can be logged.

Change-Id: I3088cde2044a7ff00841e53ca252d0b222c8b518

diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index e79c06b..647dbec 100644 (file)
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -1595,13 +1595,26 @@ class FileRepo {
         *
         * @param string $virtualUrl
         * @param array $headers Additional HTTP headers to send on success
-        * @return bool Success
+        * @return Status
+        * @since 1.27
         */
-       public function streamFile( $virtualUrl, $headers = array() ) {
+       public function streamFileWithStatus( $virtualUrl, $headers = array() ) {
                $path = $this->resolveToStoragePath( $virtualUrl );
                $params = array( 'src' => $path, 'headers' => $headers );
 
-               return $this->backend->streamFile( $params )->isOK();
+               return $this->backend->streamFile( $params );
+       }
+
+       /**
+        * Attempt to stream a file with the given virtual URL/storage path
+        *
+        * @deprecated since 1.26, use streamFileWithStatus
+        * @param string $virtualUrl
+        * @param array $headers Additional HTTP headers to send on success
+        * @return bool Success
+        */
+       public function streamFile( $virtualUrl, $headers = array() ) {
+               return $this->streamFileWithStatus( $virtualUrl, $headers )->isOK();
        }
 
        /**

diff --git a/includes/media/MediaTransformOutput.php b/includes/media/MediaTransformOutput.php
index 1dd8519..0d51c47 100644 (file)
--- a/includes/media/MediaTransformOutput.php
+++ b/includes/media/MediaTransformOutput.php
@@ -194,20 +194,32 @@ abstract class MediaTransformOutput {
         * Stream the file if there were no errors
         *
         * @param array $headers Additional HTTP headers to send on success
-        * @return bool Success
+        * @return Status
+        * @since 1.27
         */
-       public function streamFile( $headers = array() ) {
+       public function streamFileWithStatus( $headers = array() ) {
                if ( !$this->path ) {
-                       return false;
+                       return Status::newFatal( 'backend-fail-stream', '<no path>' );
                } elseif ( FileBackend::isStoragePath( $this->path ) ) {
                        $be = $this->file->getRepo()->getBackend();
-
-                       return $be->streamFile( array( 'src' => $this->path, 'headers' => $headers ) )->isOK();
+                       return $be->streamFile( array( 'src' => $this->path, 'headers' => $headers ) );
                } else { // FS-file
-                       return StreamFile::stream( $this->getLocalCopyPath(), $headers );
+                       $success = StreamFile::stream( $this->getLocalCopyPath(), $headers );
+                       return $success ? Status::newGood() : Status::newFatal( 'backend-fail-stream', $this->path );
                }
        }
 
+       /**
+        * Stream the file if there were no errors
+        *
+        * @deprecated since 1.26, use streamFileWithStatus
+        * @param array $headers Additional HTTP headers to send on success
+        * @return bool Success
+        */
+       public function streamFile( $headers = array() ) {
+               $this->streamFileWithStatus( $headers )->isOK();
+       }
+
        /**
         * Wrap some XHTML text in an anchor tag with the given attributes
         *

diff --git a/thumb.php b/thumb.php
index 34f641d..fed0258 100644 (file)
--- a/thumb.php
+++ b/thumb.php
@@ -21,6 +21,8 @@
  * @ingroup Media
  */
 
+use MediaWiki\Logger\LoggerFactory;
+
 define( 'MW_NO_OUTPUT_COMPRESSION', 1 );
 require __DIR__ . '/includes/WebStart.php';
 
@@ -262,7 +264,8 @@ function wfStreamThumb( array $params ) {
                );
                return;
        } catch ( MWException $e ) {
-               wfThumbError( 500, $e->getHTML() );
+               wfThumbError( 500, $e->getHTML(), 'Exception caught while extracting thumb name',
+                       array( 'exception' => $e ) );
                return;
        }
 
@@ -310,13 +313,14 @@ function wfStreamThumb( array $params ) {
        $thumbPath = $img->getThumbPath( $thumbName );
        if ( $img->getRepo()->fileExists( $thumbPath ) ) {
                $starttime = microtime( true );
-               $success = $img->getRepo()->streamFile( $thumbPath, $headers );
+               $status = $img->getRepo()->streamFileWithStatus( $thumbPath, $headers );
                $streamtime = microtime( true ) - $starttime;
 
-               if ( !$success ) {
-                       wfThumbError( 500, 'Could not stream the file' );
-               } else {
+               if ( $status->isOK() ) {
                        RequestContext::getMain()->getStats()->timing( 'media.thumbnail.stream', $streamtime );
+               } else {
+                       wfThumbError( 500, 'Could not stream the file', null, array( 'file' => $thumbName,
+                               'path' => $thumbPath, 'error' => $status->getWikiText() ) );
                }
                return;
        }
@@ -356,12 +360,13 @@ function wfStreamThumb( array $params ) {
        }
 
        if ( $errorMsg !== false ) {
-               wfThumbError( $errorCode, $errorMsg );
+               wfThumbError( $errorCode, $errorMsg, null, array( 'file' => $thumbName, 'path' => $thumbPath ) );
        } else {
                // Stream the file if there were no errors
-               $success = $thumb->streamFile( $headers );
-               if ( !$success ) {
-                       wfThumbError( 500, 'Could not stream the file' );
+               $status = $thumb->streamFileWithStatus( $headers );
+               if ( !$status->isOK() ) {
+                       wfThumbError( 500, 'Could not stream the file', null, array(
+                               'file' => $thumbName, 'path' => $thumbPath, 'error' => $status->getWikiText() ) );
                }
        }
 }
@@ -579,9 +584,12 @@ function wfThumbErrorText( $status, $msgText ) {
  *
  * @param int $status
  * @param string $msgHtml HTML
+ * @param string $msgText Short error description, for internal logging. Defaults to $msgHtml.
+ *   Only used for HTTP 500 errors.
+ * @param array $context Error context, for internal logging. Only used for HTTP 500 errors.
  * @return void
  */
-function wfThumbError( $status, $msgHtml ) {
+function wfThumbError( $status, $msgHtml, $msgText = null, $context = array() ) {
        global $wgShowHostnames;
 
        header( 'Cache-Control: no-cache' );
@@ -592,6 +600,7 @@ function wfThumbError( $status, $msgHtml ) {
                HttpStatus::header( 403 );
                header( 'Vary: Cookie' );
        } else {
+               LoggerFactory::getInstance( 'thumb' )->error( $msgText ?: $msgHtml, $context );
                HttpStatus::header( 500 );
        }
        if ( $wgShowHostnames ) {