Last modified: 2013-07-25 07:04:51 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T46111, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 44111 - Wikibase throws localised exceptions in ErrorPageError
Wikibase throws localised exceptions in ErrorPageError
Status: VERIFIED FIXED
Product: MediaWiki extensions
Classification: Unclassified
WikidataRepo (Other open bugs)
unspecified
All All
: Normal major (vote)
: ---
Assigned To: Wikidata bugs
: i18n
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-18 15:23 UTC by Max Semenik
Modified: 2013-07-25 07:04 UTC (History)
10 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Max Semenik 2013-01-18 15:23:13 UTC
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.
Comment 1 Daniel Kinzler 2013-01-23 19:20:01 UTC
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.
Comment 2 Daniel Kinzler 2013-01-23 20:45:59 UTC
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.
Comment 3 Max Semenik 2013-01-28 22:28:21 UTC
(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}
Comment 4 Brad Jorsch 2013-02-20 17:03:44 UTC
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().
Comment 5 Daniel Kinzler 2013-02-20 17:33:18 UTC
(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?
Comment 6 Brad Jorsch 2013-02-20 22:56:38 UTC
(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.
Comment 7 Andre Klapper 2013-03-07 19:48:27 UTC
Not sure how to proceed here - is this request invalid as per comment 4?
Comment 8 Daniel Kinzler 2013-03-07 19:53:16 UTC
@andre: perhaps it could be addressed as part of bug 45843
Comment 9 Brad Jorsch 2013-03-07 19:56:12 UTC
(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.
Comment 10 Daniel Kinzler 2013-03-07 20:15:52 UTC
Brad is right - it's related, but not the same thing.
Comment 11 Max Semenik 2013-03-10 06:34:41 UTC
Reclassifying back to  Wikibase: simply don't thorow UI-oriented exceptions from API code.
Comment 12 Daniel Kinzler 2013-03-10 19:30:28 UTC
(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.
Comment 13 Brad Jorsch 2013-03-11 18:43:18 UTC
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
Comment 14 Brad Jorsch 2013-03-11 18:45:37 UTC
(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.
Comment 15 Siebrand Mazeland 2013-04-04 07:48:35 UTC
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?
Comment 16 Andre Klapper 2013-04-25 11:48:48 UTC
(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?
Comment 17 Max Semenik 2013-04-25 14:04:12 UTC
(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.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links