Add ILocalizedException interface
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 8 Dec 2016 18:38:45 +0000 (13:38 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Thu, 8 Dec 2016 18:53:24 +0000 (13:53 -0500)
We already throw around some exceptions that are localized
(ErrorPageError and its subclasses, MalformedTitleException), but
there's no standard way to recognize them. Let's change that.

Then let's use them in the API to be able to have internationalized
errors when such exceptions are caught, instead of wrapping the
English-language version.

Change-Id: Iac7c90f92a889f8de9dae373547c07b884addaea

19 files changed:
RELEASE-NOTES-1.29
autoload.php
includes/api/ApiBase.php
includes/api/ApiEditPage.php
includes/api/ApiErrorFormatter.php
includes/api/ApiImport.php
includes/api/ApiMain.php
includes/api/ApiPageSet.php
includes/api/ApiParse.php
includes/api/ApiQueryStashImageInfo.php
includes/api/ApiUpload.php
includes/api/ApiUsageException.php
includes/api/i18n/en.json
includes/api/i18n/qqq.json
includes/exception/ErrorPageError.php
includes/exception/LocalizedException.php [new file with mode: 0644]
includes/exception/PermissionsError.php
includes/libs/rdbms/exception/DBExpectedError.php
includes/title/MalformedTitleException.php

index 986ecd8..e51dacf 100644 (file)
@@ -26,6 +26,8 @@ production.
 === New features in 1.29 ===
 * (T5233) A cookie can now be set when a user is autoblocked, to track that user if
   they move to a new IP address. This is disabled by default.
+* Added ILocalizedException interface to standardize the use of localized
+  exceptions, largely so the API can handle them more sensibly.
 
 === External library changes in 1.29 ===
 
@@ -49,6 +51,8 @@ production.
   using the new 'errorformat', 'errorlang', and 'errorsuselocal' parameters.
 * API error codes may have changed. Most notably, errors from modules using
   parameter prefixes (e.g. all query submodules) will no longer be prefixed.
+* ApiPageSet-using modules will report the 'invalidreason' using the specified
+  'errorformat'.
 * action=emailuser may return a "Warnings" status, and now returns 'warnings' and
   'errors' subelements (as applicable) instead of 'message'.
 * action=imagerotate returns an 'errors' subelement rather than 'errormessage'.
index e079686..60ce8f6 100644 (file)
@@ -598,6 +598,7 @@ $wgAutoloadLocalClasses = [
        'ILBFactory' => __DIR__ . '/includes/libs/rdbms/lbfactory/ILBFactory.php',
        'ILoadBalancer' => __DIR__ . '/includes/libs/rdbms/loadbalancer/ILoadBalancer.php',
        'ILoadMonitor' => __DIR__ . '/includes/libs/rdbms/loadmonitor/ILoadMonitor.php',
+       'ILocalizedException' => __DIR__ . '/includes/exception/LocalizedException.php',
        'IMaintainableDatabase' => __DIR__ . '/includes/libs/rdbms/database/IMaintainableDatabase.php',
        'IP' => __DIR__ . '/includes/libs/IP.php',
        'IPSet' => __DIR__ . '/includes/compat/IPSetCompat.php',
@@ -759,6 +760,7 @@ $wgAutoloadLocalClasses = [
        'LocalSettingsGenerator' => __DIR__ . '/includes/installer/LocalSettingsGenerator.php',
        'LocalisationCache' => __DIR__ . '/includes/cache/localisation/LocalisationCache.php',
        'LocalisationCacheBulkLoad' => __DIR__ . '/includes/cache/localisation/LocalisationCacheBulkLoad.php',
+       'LocalizedException' => __DIR__ . '/includes/exception/LocalizedException.php',
        'LockManager' => __DIR__ . '/includes/libs/lockmanager/LockManager.php',
        'LockManagerGroup' => __DIR__ . '/includes/filebackend/lockmanager/LockManagerGroup.php',
        'LogEntry' => __DIR__ . '/includes/logging/LogEntry.php',
index a40593f..65fcb99 100644 (file)
@@ -1723,6 +1723,20 @@ abstract class ApiBase extends ContextSource {
                throw ApiUsageException::newWithMessage( $this, $msg, $code, $data, $httpCode );
        }
 
+       /**
+        * Abort execution with an error derived from an exception
+        *
+        * @since 1.29
+        * @param Exception|Throwable $exception See ApiErrorFormatter::getMessageFromException()
+        * @param array $options See ApiErrorFormatter::getMessageFromException()
+        * @throws ApiUsageException always
+        */
+       public function dieWithException( $exception, array $options = [] ) {
+               $this->dieWithError(
+                       $this->getErrorFormatter()->getMessageFromException( $exception, $options )
+               );
+       }
+
        /**
         * Adds a warning to the output, else dies
         *
index 6b56870..e5c73b3 100644 (file)
@@ -140,11 +140,9 @@ class ApiEditPage extends ApiBase {
                                        try {
                                                $content = ContentHandler::makeContent( $text, $this->getTitle() );
                                        } catch ( MWContentSerializationException $ex ) {
-                                               // @todo: Internationalize MWContentSerializationException
-                                               $this->dieWithError(
-                                                       [ 'apierror-contentserializationexception', wfEscapeWikiText( $ex->getMessage() ) ],
-                                                       'parseerror'
-                                               );
+                                               $this->dieWithException( $ex, [
+                                                       'wrap' => ApiMessage::create( 'apierror-contentserializationexception', 'parseerror' )
+                                               ] );
                                                return;
                                        }
                                } else {
index 4fb19b8..f246203 100644 (file)
@@ -142,6 +142,66 @@ class ApiErrorFormatter {
                }
        }
 
+       /**
+        * Get an ApiMessage from an exception
+        * @since 1.29
+        * @param Exception|Throwable $exception
+        * @param array $options
+        *  - wrap: (string|array|MessageSpecifier) Used to wrap the exception's
+        *    message. The exception's message will be added as the final parameter.
+        *  - code: (string) Default code
+        *  - data: (array) Extra data
+        * @return ApiMessage
+        */
+       public function getMessageFromException( $exception, array $options = [] ) {
+               $options += [ 'code' => null, 'data' => [] ];
+
+               if ( $exception instanceof ILocalizedException ) {
+                       $msg = $exception->getMessageObject();
+                       $params = [];
+               } else {
+                       // Extract code and data from the exception, if applicable
+                       if ( $exception instanceof UsageException ) {
+                               $data = $exception->getMessageArray();
+                               if ( !isset( $options['code'] ) ) {
+                                       $options['code'] = $data['code'];
+                               }
+                               unset( $data['code'], $data['info'] );
+                               $options['data'] = array_merge( $data['code'], $options['data'] );
+                       }
+
+                       if ( isset( $options['wrap'] ) ) {
+                               $msg = $options['wrap'];
+                       } else {
+                               $msg = new RawMessage( '$1' );
+                               if ( !isset( $options['code'] ) ) {
+                                       $options['code'] = 'internal_api_error_' . get_class( $exception );
+                               }
+                       }
+                       $params = [ wfEscapeWikiText( $exception->getMessage() ) ];
+               }
+               return ApiMessage::create( $msg, $options['code'], $options['data'] )
+                       ->params( $params )
+                       ->inLanguage( $this->lang )
+                       ->title( $this->getDummyTitle() )
+                       ->useDatabase( $this->useDB );
+       }
+
+       /**
+        * Format an exception as an array
+        * @since 1.29
+        * @param Exception|Throwable $exception
+        * @param array $options See self::getMessageFromException(), plus
+        *  - format: (string) Format override
+        * @return array
+        */
+       public function formatException( $exception, array $options = [] ) {
+               return $this->formatMessage(
+                       $this->getMessageFromException( $exception, $options ),
+                       isset( $options['format'] ) ? $options['format'] : null
+               );
+       }
+
        /**
         * Format a message as an array
         * @param Message|array|string $msg Message. See ApiMessage::create().
@@ -335,6 +395,19 @@ class ApiErrorFormatter_BackCompat extends ApiErrorFormatter {
                ] + $msg->getApiData();
        }
 
+       /**
+        * Format an exception as an array
+        * @since 1.29
+        * @param Exception|Throwable $exception
+        * @param array $options See parent::formatException(), plus
+        *  - bc: (bool) Return only the string, not an array
+        * @return array|string
+        */
+       public function formatException( $exception, array $options = [] ) {
+               $ret = parent::formatException( $exception, $options );
+               return empty( $options['bc'] ) ? $ret : $ret['info'];
+       }
+
        protected function addWarningOrError( $tag, $modulePath, $msg ) {
                $value = self::stripMarkup( $msg->text() );
 
index 3f48f38..dffd6b2 100644 (file)
@@ -83,7 +83,7 @@ class ApiImport extends ApiBase {
                try {
                        $importer->doImport();
                } catch ( Exception $e ) {
-                       $this->dieWithError( [ 'apierror-import-unknownerror', wfEscapeWikiText( $e->getMessage() ) ] );
+                       $this->dieWithException( $e, [ 'wrap' => 'apierror-import-unknownerror' ] );
                }
 
                $resultData = $reporter->getData();
index fe6ed41..54679a8 100644 (file)
@@ -1046,7 +1046,9 @@ class ApiMain extends ApiBase {
                                $params = [
                                        'apierror-exceptioncaught',
                                        WebRequest::getRequestId(),
-                                       wfEscapeWikiText( $e->getMessage() )
+                                       $e instanceof ILocalizedException
+                                               ? $e->getMessageObject()
+                                               : wfEscapeWikiText( $e->getMessage() )
                                ];
                        }
                        $messages[] = ApiMessage::create( $params, $code );
index 4cf896f..160ce87 100644 (file)
@@ -1151,7 +1151,7 @@ class ApiPageSet extends ApiBase {
                                        $this->mAllPages[0][$title] = $this->mFakePageId;
                                        $this->mInvalidTitles[$this->mFakePageId] = [
                                                'title' => $title,
-                                               'invalidreason' => $ex->getMessage(),
+                                               'invalidreason' => $this->getErrorFormatter()->formatException( $ex, [ 'bc' => true ] ),
                                        ];
                                        $this->mFakePageId--;
                                        continue; // There's nothing else we can do
index 2263b8f..287ffb7 100644 (file)
@@ -225,11 +225,9 @@ class ApiParse extends ApiBase {
                        try {
                                $this->content = ContentHandler::makeContent( $text, $titleObj, $model, $format );
                        } catch ( MWContentSerializationException $ex ) {
-                               // @todo: Internationalize MWContentSerializationException
-                               $this->dieWithError(
-                                       [ 'apierror-contentserializationexception', wfEscapeWikiText( $ex->getMessage() ) ],
-                                       'parseerror'
-                               );
+                               $this->dieWithException( $ex, [
+                                       'wrap' => ApiMessage::create( 'apierror-contentserializationexception', 'parseerror' )
+                               ] );
                        }
 
                        if ( $this->section !== false ) {
index 981cb09..abb827f 100644 (file)
@@ -63,11 +63,10 @@ class ApiQueryStashImageInfo extends ApiQueryImageInfo {
                                $result->addIndexedTagName( [ 'query', $this->getModuleName() ], $modulePrefix );
                        }
                // @todo Update exception handling here to understand current getFile exceptions
-               // @todo Internationalize the exceptions
                } catch ( UploadStashFileNotFoundException $e ) {
-                       $this->dieWithError( [ 'apierror-stashedfilenotfound', wfEscapeWikiText( $e->getMessage() ) ] );
+                       $this->dieWithException( $e, [ 'wrap' => 'apierror-stashedfilenotfound' ] );
                } catch ( UploadStashBadPathException $e ) {
-                       $this->dieWithError( [ 'apierror-stashpathinvalid', wfEscapeWikiText( $e->getMessage() ) ] );
+                       $this->dieWithException( $e, [ 'wrap' => 'apierror-stashpathinvalid' ] );
                }
        }
 
index 6bdd68f..311fa54 100644 (file)
@@ -320,12 +320,14 @@ class ApiUpload extends ApiBase {
 
                        if ( $status->isGood() && !$status->getValue() ) {
                                // Not actually a 'good' status...
-                               $status->fatal( new ApiRawMessage( 'Invalid stashed file', 'stashfailed' ) );
+                               $status->fatal( new ApiMessage( 'apierror-stashinvalidfile', 'stashfailed' ) );
                        }
                } catch ( Exception $e ) {
                        $debugMessage = 'Stashing temporary file failed: ' . get_class( $e ) . ' ' . $e->getMessage();
                        wfDebug( __METHOD__ . ' ' . $debugMessage . "\n" );
-                       $status = Status::newFatal( new ApiRawMessage( $e->getMessage(), 'stashfailed' ) );
+                       $status = Status::newFatal( $this->getErrorFormatter()->getMessageFromException(
+                               $e, [ 'wrap' => new ApiMessage( 'apierror-stashexception', 'stashfailed' ) ]
+                       ) );
                }
 
                if ( $status->isGood() ) {
@@ -564,7 +566,6 @@ class ApiUpload extends ApiBase {
         * @param array $verification
         */
        protected function checkVerification( array $verification ) {
-               // @todo Move them to ApiBase's message map
                switch ( $verification['status'] ) {
                        // Recoverable errors
                        case UploadBase::MIN_LENGTH_PARTNAME:
@@ -713,32 +714,41 @@ class ApiUpload extends ApiBase {
 
        /**
         * Handles a stash exception, giving a useful error to the user.
-        * @todo Internationalize the exceptions
+        * @todo Internationalize the exceptions then get rid of this
         * @param Exception $e
         * @return StatusValue
         */
        protected function handleStashException( $e ) {
-               $err = wfEscapeWikiText( $e->getMessage() );
                switch ( get_class( $exception ) ) {
                        case 'UploadStashFileNotFoundException':
-                               return StatusValue::newFatal( 'apierror-stashedfilenotfound', $err );
+                               $wrap = 'apierror-stashedfilenotfound';
+                               break;
                        case 'UploadStashBadPathException':
-                               return StatusValue::newFatal( 'apierror-stashpathinvalid', $err );
+                               $wrap = 'apierror-stashpathinvalid';
+                               break;
                        case 'UploadStashFileException':
-                               return StatusValue::newFatal( 'apierror-stashfilestorage', $err );
+                               $wrap = 'apierror-stashfilestorage';
+                               break;
                        case 'UploadStashZeroLengthFileException':
-                               return StatusValue::newFatal( 'apierror-stashzerolength', $err );
+                               $wrap = 'apierror-stashzerolength';
+                               break;
                        case 'UploadStashNotLoggedInException':
                                return StatusValue::newFatal( ApiMessage::create(
                                        [ 'apierror-mustbeloggedin', $this->msg( 'action-upload' ) ], 'stashnotloggedin'
                                ) );
                        case 'UploadStashWrongOwnerException':
-                               return StatusValue::newFatal( 'apierror-stashwrongowner', $err );
+                               $wrap = 'apierror-stashwrongowner';
+                               break;
                        case 'UploadStashNoSuchKeyException':
-                               return StatusValue::newFatal( 'apierror-stashnosuchfilekey', $err );
+                               $wrap = 'apierror-stashnosuchfilekey';
+                               break;
                        default:
-                               return StatusValue::newFatal( 'uploadstash-exception', get_class( $e ), $err );
+                               $wrap = [ 'uploadstash-exception', get_class( $e ) ];
+                               break;
                }
+               return StatusValue::newFatal(
+                       $this->getErrorFormatter()->getMessageFromException( $e, [ 'wrap' => $wrap ] )
+               );
        }
 
        /**
index 7e21ab5..9dc1f92 100644 (file)
@@ -95,7 +95,7 @@ class UsageException extends MWException {
  *  starts throwing ApiUsageException. Eventually UsageException will go away
  *  and this will (probably) extend MWException directly.
  */
-class ApiUsageException extends UsageException {
+class ApiUsageException extends UsageException implements ILocalizedException {
 
        protected $modulePath;
        protected $status;
@@ -201,6 +201,13 @@ class ApiUsageException extends UsageException {
                ] + $enMsg->getApiData();
        }
 
+       /**
+        * @inheritdoc
+        */
+       public function getMessageObject() {
+               return $this->status->getMessage();
+       }
+
        /**
         * @return string
         */
index 02aa6db..aa9e4ac 100644 (file)
        "apierror-specialpage-cantexecute": "You don't have permission to view the results of this special page.",
        "apierror-stashedfilenotfound": "Could not find the file in the stash: $1.",
        "apierror-stashedit-missingtext": "No stashed text found with the given hash.",
+       "apierror-stashexception": "$1",
        "apierror-stashfailed-complete": "Chunked upload is already completed, check status for details.",
        "apierror-stashfailed-nosession": "No chunked upload session with this key.",
        "apierror-stashfilestorage": "Could not store upload in the stash: $1",
+       "apierror-stashinvalidfile": "Invalid stashed file.",
        "apierror-stashnosuchfilekey": "No such filekey: $1.",
        "apierror-stashpathinvalid": "File key of improper format or otherwise invalid: $1.",
        "apierror-stashwrongowner": "Wrong owner: $1",
index 8cf338d..caf202c 100644 (file)
        "apierror-stashedfilenotfound": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text. Currently this is probably English, hopefully we'll fix that in the future.",
        "apierror-stashedit-missingtext": "{{doc-apierror}}",
        "apierror-stashfailed-complete": "{{doc-apierror}}",
+       "apierror-stashexception": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text. May be English or localized, may or may not end in punctuation.",
        "apierror-stashfailed-nosession": "{{doc-apierror}}",
        "apierror-stashfilestorage": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text, which may already end with punctuation. Currently this is probably English, hopefully we'll fix that in the future.",
+       "apierror-stashinvalidfile": "{{doc-apierror}}",
        "apierror-stashnosuchfilekey": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text. Currently this is probably English, hopefully we'll fix that in the future.",
        "apierror-stashpathinvalid": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text. Currently this is probably English, hopefully we'll fix that in the future.",
        "apierror-stashwrongowner": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text, which should already end with punctuation. Currently this is probably English, hopefully we'll fix that in the future.",
index 2f502d8..9b5a268 100644 (file)
@@ -24,7 +24,7 @@
  * @since 1.7
  * @ingroup Exception
  */
-class ErrorPageError extends MWException {
+class ErrorPageError extends MWException implements ILocalizedException {
        public $title, $msg, $params;
 
        /**
@@ -43,15 +43,23 @@ class ErrorPageError extends MWException {
                // customized by the local wiki. So get the default English version for
                // passing to the parent constructor. Our overridden report() below
                // makes sure that the page shown to the user is not forced to English.
-               if ( $msg instanceof Message ) {
-                       $enMsg = clone $msg;
-               } else {
-                       $enMsg = wfMessage( $msg, $params );
-               }
+               $enMsg = $this->getMessageObject();
                $enMsg->inLanguage( 'en' )->useDatabase( false );
                parent::__construct( $enMsg->text() );
        }
 
+       /**
+        * Return a Message object for this exception
+        * @since 1.29
+        * @return Message
+        */
+       public function getMessageObject() {
+               if ( $this->msg instanceof Message ) {
+                       return clone $this->msg;
+               }
+               return wfMessage( $this->msg, $this->params );
+       }
+
        public function report() {
                global $wgOut;
 
diff --git a/includes/exception/LocalizedException.php b/includes/exception/LocalizedException.php
new file mode 100644 (file)
index 0000000..cbdb53e
--- /dev/null
@@ -0,0 +1,66 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Interface for MediaWiki-localized exceptions
+ *
+ * @since 1.29
+ * @ingroup Exception
+ */
+interface ILocalizedException {
+       /**
+        * Return a Message object for this exception
+        * @return Message
+        */
+       public function getMessageObject();
+}
+
+/**
+ * Basic localized exception.
+ *
+ * @since 1.29
+ * @ingroup Exception
+ * @note Don't use this in a situation where MessageCache is not functional.
+ */
+class LocalizedException extends Exception implements ILocalizedException {
+       /** @var string|array|MessageSpecifier */
+       protected $messageSpec;
+
+       /**
+        * @param string|array|MessageSpecifier $messageSpec See Message::newFromSpecifier
+        * @param int $code Exception code
+        * @param Exception|Throwable $previous The previous exception used for the exception chaining.
+        */
+       public function __construct( $messageSpec, $code = 0, $previous = null ) {
+               $this->messageSpec = $messageSpec;
+
+               // Exception->getMessage() should be in plain English, not localized.
+               // So fetch the English version of the message, without local
+               // customizations, and make a basic attempt to turn markup into text.
+               $msg = $this->getMessageObject()->inLanguage( 'en' )->useDatabase( false )->text();
+               $msg = preg_replace( '!</?(var|kbd|samp|code)>!', '"', $msg );
+               $msg = html_entity_decode( strip_tags( $msg ), ENT_QUOTES | ENT_HTML5 );
+               parent::__construct( $msg, $code, $previous );
+       }
+
+       public function getMessageObject() {
+               return Message::newFromSpecifier( $this->messageSpec );
+       }
+}
index e31374c..5ecd237 100644 (file)
@@ -58,6 +58,9 @@ class PermissionsError extends ErrorPageError {
                }
 
                $this->errors = $errors;
+
+               // Give the parent class something to work with
+               parent::__construct( 'permissionserrors', Message::newFromSpecifier( $errors[0] ) );
        }
 
        public function report() {
index 9e10884..7d303b1 100644 (file)
@@ -26,7 +26,7 @@
  * @ingroup Database
  * @since 1.23
  */
-class DBExpectedError extends DBError implements MessageSpecifier {
+class DBExpectedError extends DBError implements MessageSpecifier, ILocalizedException {
        /** @var string[] Message parameters */
        protected $params;
 
@@ -42,4 +42,12 @@ class DBExpectedError extends DBError implements MessageSpecifier {
        public function getParams() {
                return $this->params;
        }
+
+       /**
+        * @inheritdoc
+        * @since 1.29
+        */
+       public function getMessageObject() {
+               return Message::newFromSpecifier( $this );
+       }
 }
index 799961a..2dddac5 100644 (file)
@@ -22,7 +22,7 @@
  * MalformedTitleException is thrown when a TitleParser is unable to parse a title string.
  * @since 1.23
  */
-class MalformedTitleException extends Exception {
+class MalformedTitleException extends Exception implements ILocalizedException {
        private $titleText = null;
        private $errorMessage = null;
        private $errorMessageParameters = [];
@@ -72,4 +72,12 @@ class MalformedTitleException extends Exception {
        public function getErrorMessageParameters() {
                return $this->errorMessageParameters;
        }
+
+       /**
+        * @since 1.29
+        * @return Message
+        */
+       public function getMessageObject() {
+               return wfMessage( $this->getErrorMessage(), $this->getErrorMessageParameters() );
+       }
 }