Merge "Add more detailed upload stash error messages"

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index f3add13..9ddadcb 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 ceb5b66..b62c4a5 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'
        ];
 
        /**