Add more detailed upload stash error messages

There are seven (used) error types in the stash class, and we umbrella'd
them all into one error message, which is mighty silly. This should give
us more information.

Also added to the mw.Api.errors list so UploadWizard can handle them.

Change-Id: I79bf0c29a4cef19363d111cc1128e35256ae572a

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 8cf53d7..eadc95f 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -64,7 +64,7 @@ class ApiUpload extends ApiBase {
                                $this->dieUsage( 'No upload module set', 'nomodule' );
                        }
                } catch ( UploadStashException $e ) { // XXX: don't spam exception log
-                       $this->dieUsage( get_class( $e ) . ": " . $e->getMessage(), 'stasherror' );
+                       $this->handleStashException( $e );
                }
 
                // First check permission to upload
@@ -112,7 +112,7 @@ class ApiUpload extends ApiBase {
                                $result['imageinfo'] = $this->mUpload->getImageInfo( $this->getResult() );
                        }
                } catch ( UploadStashException $e ) { // XXX: don't spam exception log
-                       $this->dieUsage( get_class( $e ) . ": " . $e->getMessage(), 'stasherror' );
+                       $this->handleStashException( $e );
                }
 
                $this->getResult()->addValue( null, $this->getModuleName(), $result );
@@ -159,6 +159,8 @@ class ApiUpload extends ApiBase {
                        if ( $warnings && count( $warnings ) > 0 ) {
                                $result['warnings'] = $warnings;
                        }
+               } catch ( UploadStashException $e ) {
+                       $this->handleStashException( $e );
                } catch ( MWException $e ) {
                        $this->dieUsage( $e->getMessage(), 'stashfailed' );
                }
@@ -180,6 +182,8 @@ class ApiUpload extends ApiBase {
                try {
                        $result['filekey'] = $this->performStash();
                        $result['sessionkey'] = $result['filekey']; // backwards compatibility
+               } catch ( UploadStashException $e ) {
+                       $this->handleStashException( $e );
                } catch ( MWException $e ) {
                        $result['warnings']['stashfailed'] = $e->getMessage();
                }
@@ -205,6 +209,8 @@ class ApiUpload extends ApiBase {
                if ( $this->mParams['offset'] == 0 ) {
                        try {
                                $filekey = $this->performStash();
+                       } catch ( UploadStashException $e ) {
+                               $this->handleStashException( $e );
                        } catch ( MWException $e ) {
                                // FIXME: Error handling here is wrong/different from rest of this
                                $this->dieUsage( $e->getMessage(), 'stashfailed' );
@@ -282,7 +288,8 @@ class ApiUpload extends ApiBase {
                } catch ( MWException $e ) {
                        $message = 'Stashing temporary file failed: ' . get_class( $e ) . ' ' . $e->getMessage();
                        wfDebug( __METHOD__ . ' ' . $message . "\n" );
-                       throw new MWException( $message );
+                       $className = get_class( $e );
+                       throw new $className( $message );
                }
 
                return $fileKey;
@@ -576,6 +583,41 @@ class ApiUpload extends ApiBase {
                return $warnings;
        }
 
+       /**
+        * Handles a stash exception, giving a useful error to the user.
+        * @param Exception $e The exception we encountered.
+        */
+       protected function handleStashException( $e ) {
+               $exceptionType = get_class( $e );
+
+               switch ( $exceptionType ) {
+                       case 'UploadStashFileNotFoundException':
+                               $this->dieUsage( 'Could not find the file in the stash: ' . $e->getMessage(), 'stashedfilenotfound' );
+                               break;
+                       case 'UploadStashBadPathException':
+                               $this->dieUsage( 'File key of improper format or otherwise invalid: ' . $e->getMessage(), 'stashpathinvalid' );
+                               break;
+                       case 'UploadStashFileException':
+                               $this->dieUsage( 'Could not store upload in the stash: ' . $e->getMessage(), 'stashfilestorage' );
+                               break;
+                       case 'UploadStashZeroLengthFileException':
+                               $this->dieUsage( 'File is of zero length, and could not be stored in the stash: ' . $e->getMessage(), 'stashzerolength' );
+                               break;
+                       case 'UploadStashNotLoggedInException':
+                               $this->dieUsage( 'Not logged in: ' . $e->getMessage(), 'stashnotloggedin' );
+                               break;
+                       case 'UploadStashWrongOwnerException':
+                               $this->dieUsage( 'Wrong owner: ' . $e->getMessage(), 'stashwrongowner' );
+                               break;
+                       case 'UploadStashNoSuchKeyException':
+                               $this->dieUsage( 'No such filekey: ' . $e->getMessage(), 'stashnosuchfilekey' );
+                               break;
+                       default:
+                               $this->dieUsage( $exceptionType . ": " . $e->getMessage(), 'stasherror' );
+                               break;
+               }
+       }
+
        /**
         * Perform the actual upload. Returns a suitable result array on success;
         * dies on failure.

diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index 7d80b44..52ce4d3 100644 (file)
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
@@ -151,6 +151,7 @@ class UploadStash {
 
                if ( !$this->files[$key]->exists() ) {
                        wfDebug( __METHOD__ . " tried to get file at $key, but it doesn't exist\n" );
+                       // @todo Is this not an UploadStashFileNotFoundException case?
                        throw new UploadStashBadPathException( "path doesn't exist" );
                }
 

diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index 1c6890c..341f626 100644 (file)
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -3484,6 +3484,13 @@
        "api-error-stashfailed": "Internal error: Server failed to store temporary file.",
        "api-error-publishfailed": "Internal error: Server failed to publish temporary file.",
        "api-error-stasherror": "There was an error while uploading the file to stash.",
+       "api-error-stashedfilenotfound": "The stashed file was not found when attempting to upload it from the stash.",
+       "api-error-stashpathinvalid": "The path at which the stashed file should have been found was invalid.",
+       "api-error-stashfilestorage": "There was an error while storing the file in the stash.",
+       "api-error-stashzerolength": "The server could not stash the file, because it had zero length.",
+       "api-error-stashnotloggedin": "You must be logged in to save files in the upload stash.",
+       "api-error-stashwrongowner": "The file you were attempting to access in the stash does not belong to you.",
+       "api-error-stashnosuchfilekey": "The file key you were attempting to access in the stash does not exist.",
        "api-error-timeout": "The server did not respond within the expected time.",
        "api-error-unclassified": "An unknown error occurred.",
        "api-error-unknown-code": "Unknown error: \"$1\".",

diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index c432adf..ccb481e 100644 (file)
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -75,6 +75,7 @@
                        "MIKHEIL",
                        "Malafaya",
                        "MarkvA",
+                       "marktraceur",
                        "Matma Rex",
                        "MaxSem",
                        "McDutchie",
@@ -3647,6 +3648,13 @@
        "api-error-stashfailed": "API error message that can be used for client side localisation of API errors.",
        "api-error-publishfailed": "API error message that can be used for client side localisation of API errors.",
        "api-error-stasherror": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashedfilenotfound": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashpathinvalid": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashfilestorage": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashzerolength": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashnotloggedin": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashwrongowner": "API error message that can be used for client side localisation of API errors.",
+       "api-error-stashnosuchfilekey": "API error message that can be used for client side localisation of API errors.",
        "api-error-timeout": "API error message that can be used for client side localisation of API errors.",
        "api-error-unclassified": "API error message that can be used for client side localisation of API errors.",
        "api-error-unknown-code": "API error message that can be used for client side localisation of API errors.\n\nParameters:\n* $1 - may contain more error details\n{{Identical|Unknown error}}",

diff --git a/resources/src/mediawiki.api/mediawiki.api.js b/resources/src/mediawiki.api/mediawiki.api.js
index bb0642e..fb6982c 100644 (file)
--- a/resources/src/mediawiki.api/mediawiki.api.js
+++ b/resources/src/mediawiki.api/mediawiki.api.js
@@ -320,7 +320,6 @@
                'nomodule',
                'mustbeposted',
                'badaccess-groups',
-               'stashfailed',
                'missingresult',
                'missingparam',
                'invalid-file-key',
@@ -342,7 +341,18 @@
                'fetchfileerror',
                'fileexists-shared-forbidden',
                'invalidtitle',
-               'notloggedin'
+               'notloggedin',
+
+               // Stash-specific errors - expanded
+               'stashfailed',
+               'stasherror',
+               'stashedfilenotfound',
+               'stashpathinvalid',
+               'stashfilestorage',
+               'stashzerolength',
+               'stashnotloggedin',
+               'stashwrongowner',
+               'stashnosuchfilekey'
        ];
 
        /**