Page MenuHomePhabricator

UploadWizard errors (for upload API stash failures) are a mix of two languages
Closed, ResolvedPublic

Description

During a file upload I hit the following error:

Could not store upload in the stash (UploadStashFileException): "Error storing file in '/tmp/Kp3Nxz': O eroare necunoscută s-a produs în suportul de stocare „local-swift-eqiad”.".

As you can see, the error is partly in English, partly in Romanian, despite the fact that I forced the language to en (in order to report the error): UW URL.

If I set the language to "uselang=ro", the error is different, but still in 2 languages:

Nu pot stoca încărcare în spațiul temporar (UploadStashFileException): "Error storing file in '/tmp/wWhiuz': Imposibil de conectat la suportul de stocare „local-swift-eqiad”."

The whole error message should be in a single language if the message is localized.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Strainu renamed this task from UploadWizard translation erro to UploadWizard translation errors.Jan 6 2017, 4:55 PM
Aklapper renamed this task from UploadWizard translation errors to UploadWizard errors are a mix of two languages.Jan 6 2017, 5:56 PM

The fact that you get the error is T154780 and a separate problem. But we've changed how various error messages are generated recently, this might be a regression.

Yes, I agree, the error is not the object of the bug, only the fact that the error message is bilingual.

matmarex triaged this task as Medium priority.Jan 6 2017, 11:14 PM
matmarex added a project: MediaWiki-Action-API.

This is probably a bug in the upload API, since we've recently switched UploadWizard to just displaying the API output for errors. And I think it's limited to stash failures, since the way these errors are produced is really gnarly. (If you see this for any other kind of errors, please mention it.) These shouldn't really be appearing under normal circumstances, so this isn't high priority probably.

Anomie subscribed.

The API is just passing through the error it gets from the underlying Upload code. The API has control of only the first bit, "Could not store upload in the stash (UploadStashFileException):" or "Nu pot stoca încărcare în spațiul temporar (UploadStashFileException):", which is the part that's being localized correctly.

The rest of the error is coming from the UploadStash class, specifically this code:

if ( !$storeStatus->isOK() ) {
    // It is a convention in MediaWiki to only return one error per API
    // exception, even if multiple errors are available. We use reset()
    // to pick the "first" thing that was wrong, preferring errors to
    // warnings. This is a bit lame, as we may have more info in the
    // $storeStatus and we're throwing it away, but to fix it means
    // redesigning API errors significantly.
    // $storeStatus->value just contains the virtual URL (if anything)
    // which is probably useless to the caller.
    $error = $storeStatus->getErrorsArray();
    $error = reset( $error );
    if ( !count( $error ) ) {
        $error = $storeStatus->getWarningsArray();
        $error = reset( $error );
        if ( !count( $error ) ) {
            $error = [ 'unknown', 'no error recorded' ];
        }
    }
    // At this point, $error should contain the single "most important"
    // error, plus any parameters.
    $errorMsg = array_shift( $error );
    throw new UploadStashFileException( "Error storing file in '$path': "
        . wfMessage( $errorMsg, $error )->text() );
}

You can see there at the end both the always-English "Error storing file in '$path':" bit and the wfMessage( ... )->text() bit that's going to be using $wgLang instead of the error language set in the API call.

The solution is to rework the various Upload*Exception classes to implement ILocalizedException (and do it correctly, pass the Message object as a parameter to the wrapper Message instead of converting it to text). ApiUpload is already set up to handle getting an ILocalizedException by outputting the exception's message directly without prefixing it with "Could not store upload in the stash (UploadStashFileException):".

matmarex renamed this task from UploadWizard errors are a mix of two languages to UploadWizard errors (for upload API stash failures) are a mix of two languages.Jan 19 2017, 4:21 PM

Change 383324 had a related patch set uploaded (by Cparle; owner: Cparle):
[mediawiki/core@master] Rework Upload*Exception classes to implement ILocalizedException

https://gerrit.wikimedia.org/r/383324

Change 383324 merged by jenkins-bot:
[mediawiki/core@master] Rework Upload*Exception classes to implement ILocalizedException

https://gerrit.wikimedia.org/r/383324