From 7585e214d2a0e589b43c1b503207fd27a6e84af0 Mon Sep 17 00:00:00 2001 From: Mark Holmquist Date: Fri, 24 Oct 2014 14:33:05 -0500 Subject: [PATCH] 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 --- includes/api/ApiUpload.php | 48 ++++++++++++++++++-- includes/upload/UploadStash.php | 1 + languages/i18n/en.json | 7 +++ languages/i18n/qqq.json | 8 ++++ resources/src/mediawiki.api/mediawiki.api.js | 14 +++++- 5 files changed, 73 insertions(+), 5 deletions(-) diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 8cf53d7944..eadc95f1da 100644 --- 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 7d80b4489e..52ce4d3a8a 100644 --- 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 1c6890c5cd..341f62634c 100644 --- 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 c432adf69b..ccb481e507 100644 --- 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 bb0642e0ea..fb6982ce34 100644 --- 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' ]; /** -- 2.20.1