Page MenuHomePhabricator

Wikibase throws localised exceptions in ErrorPageError
Closed, ResolvedPublic

Description

Exception from line 579 of /usr/local/apache/common-local/php-1.21wmf8/extensions/Wikibase/repo/includes/EditEntity.php: Les ajouts et mises à jour de la base de données sont actuellement bloqués, probablement pour permettre la maintenance de la base, après quoi, tout rentrera dans l'ordre.

While it's perfectly fine to localise messages displayed to users, all logged exception messages must be in English as they're for engineers fixing problems. Ypu don't want Wikipedia to be down while someone who can fix it tries to read Chinese.


Version: unspecified
Severity: major

Details

Reference
bz44111

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:24 AM
bzimport set Reference to bz44111.
bzimport added a subscriber: Unknown Object (MLST).

The code in question is:

		if ( wfReadOnly() ) {
			throw new \ReadOnlyError();
		}

I don't think there is anything wrong with that. I suppose the problem is that this exception is not caught and handled as expected in some cases. My guess is API calls.

Actually, can you provide the request that triggered this? I can't see how an API request or a request to a special page or action could not catch and handle this exception.

(In reply to comment #2)

Actually, can you provide the request that triggered this? I can't see how an
API request or a request to a special page or action could not catch and
handle
this exception.

#0 /usr/local/apache/common-local/php-1.21wmf8/extensions/Wikibase/repo/includes/api/ApiModifyEntity.php(196): Wikibase\EditEntity->attemptSave(...)
#1 /usr/local/apache/common-local/php-1.21wmf8/includes/api/ApiMain.php(826): Wikibase\ApiModifyEntity->execute()
#2 /usr/local/apache/common-local/php-1.21wmf8/includes/api/ApiMain.php(373): ApiMain->executeAction()
#3 /usr/local/apache/common-local/php-1.21wmf8/includes/api/ApiMain.php(350): ApiMain->executeActionWithErrorHandling()
#4 /usr/local/apache/common-local/php-1.21wmf8/api.php(77): ApiMain->execute()
#5 /usr/local/apache/common-local/live-1.5/api.php(3): require('/usr/local/apac...')
#6 {main}

This isn't an API bug, unless the bug is "API logs ErrorPageError exceptions" (these aren't logged for normal pageviews).

In general, if we want English in log messages then the exception's getLogMessage() shouldn't be returning non-English text in the first place. For the case of ErrorPageError, this could be fixed by having ErrorPageError::construct() call ->inLanguage( 'en' ) and probably ->useDatabase( false ) on its Message object to get the string to pass to parent::construct().

(In reply to comment #4)

For
the case of ErrorPageError, this could be fixed by having
ErrorPageError::construct() call ->inLanguage( 'en' ) and probably
->useDatabase( false ) on its Message object to get the string to pass to
parent::
construct().

But wouldn't that mean that users on the wiki would also see the English message?

(In reply to comment #5)

But wouldn't that mean that users on the wiki would also see the English
message?

If ErrorPageError::__construct was passed a Message object, we'd need to clone it before changing its properties. But if it was passed a string then ErrorPageError::render() creates a fresh Message object anyway so it seems to work fine in a quick test.

Not sure how to proceed here - is this request invalid as per comment 4?

@Andre: perhaps it could be addressed as part of bug 45843

(In reply to comment #7)

Not sure how to proceed here - is this request invalid as per comment 4?

IMO, it's a bug against the ErrorPageError class rather than the API.

Bug 45843 is about the response to API clients, not what gets logged to the server logs by the API.

Brad is right - it's related, but not the same thing.

Reclassifying back to Wikibase: simply don't thorow UI-oriented exceptions from API code.

(In reply to comment #11)

Reclassifying back to Wikibase: simply don't thorow UI-oriented exceptions
from API code.

The Exception isn't thrown by the UI code, it's triggered further inside the core.

We could of cause catch exceptions that for some reason seem UI related and try to re-interpret them somehow. But that seems nasty. Also, this issue isn't specific to Wikibase at all.

So, it seems wrong to assign it to Wikibase. Essentially, the whole error reporting scheme used by the API urgently needs work, see https://bugzilla.wikimedia.org/show_bug.cgi?id=45843, especially comment 5.

I still think it's a bug that ErrorPageError will log in the user's language. Since no one else seems to want to fix that, I guess I have to.

Gerrit change I9a6ab43d

(In reply to comment #12)

see
https://bugzilla.wikimedia.org/show_bug.cgi?id=45843, especially comment 5.

That's not a bad idea too. But ErrorPageError should still log in the correct language.

This issue has a "patch-in-gerrit" keyword, and the patch has been merged. However, that didn't resolve the issue, did it? Can someone familiar with this issue update it?

(In reply to comment #15 by Siebrand)

This issue has a "patch-in-gerrit" keyword, and the patch has been merged.
However, that didn't resolve the issue, did it? Can someone familiar with
this issue update it?

maxsem: This is still an issue, I assume?

(In reply to comment #16)

maxsem: This is still an issue, I assume?

Haven't seen this exception in the latest logs, but https://gerrit.wikimedia.org/r/#/c/53191/ should've fixed the originally reported issue.