From c2306755852e9206c7b85221209138d586e8c0ee Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Sat, 29 Aug 2015 17:13:04 -0700 Subject: [PATCH] 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 --- includes/filerepo/FileRepo.php | 19 +++++++++++++--- includes/media/MediaTransformOutput.php | 24 +++++++++++++++----- thumb.php | 29 ++++++++++++++++--------- 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index e79c06b8c2..647dbece19 100644 --- 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 1dd8519155..0d51c47635 100644 --- 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', '' ); } 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 34f641d1ea..fed025874b 100644 --- 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 ) { -- 2.20.1