Page MenuHomePhabricator

Trying to format messages in the current user's language in no-session entry points should fail consistently for every format
Closed, InvalidPublic

Description

(…) so IIUC now $message->parse() is safe to use in a no-session environment but $message->plain() is not. We should probably fix that.

Fix it in what direction?

The stack trace above looks like a bug in the MetricsPlatform extension to me - it shouldn't be trying to format messages in the current user's language when there isn't really a current user. (And it looks like it's just trying to format a debug log message, which should probably be hardcoded to English, so that's quite clearly a bug.)

Indeed. It is unsafe to unconditionally depend on global state from a Message, Status, or RequestContext object outside e.g. UI code inside a SpecialPage subclass.

The relevant call chain starts at MediaWiki\Extension\EventLogging\Hooks::getModuleData (for the ext.eventLogging/data.json file), and eventually reaches into MetricsPlatform where it formats a Message in a way that assumes an index.php UI request with a user session and language preference.

Trying to summarize the comments above, it seems that trying to format messages in the current user's language (e.g. using wfMessage() or RequestContext::getMain()->msg(), without setting the language) in no-session entry points (e.g. load.php) should fail consistently for every format (->parse(), ->plain(), and others).

I suggest we make it a non-fatal error first (that is: log it using LoggerInterface::error(), instead of throwing) and fall back to content language or other defaults. Once we no longer have any of these errors in production, we change it it into a thrown exception.

Event Timeline

(…) so IIUC now $message->parse() is safe to use in a no-session environment but $message->plain() is not. We should probably fix that.

On a closer look, I think that's wrong. Message::format() calls Message::getLanguage() early on, so unless the language was set explicitly, that will throw. The MessageParser logic is not related to that; it will get the current user for parser options (so that would run into session user unavailability issues regardless of whether the message language was explicitly specified), but silently fall back to the anon user if the session is not ready or not allowed. I don't know enough about the parser to really have an opinion on that, but the Message logic seems consistent and reasonable (throws iff the language was not specified, regardless of format).

Sorry for misreading.