Page MenuHomePhabricator

Fatal error: Class undefined: ReplyAction
Closed, ResolvedPublic

Description

Fatal error: Class undefined: ReplyAction in /srv/mediawiki/php-1.27.0-wmf.17/includes/actions/Action.php on line 99

Event Timeline

mmodell raised the priority of this task from to Needs Triage.
mmodell updated the task description. (Show Details)
mmodell added a subscriber: mmodell.
demon triaged this task as High priority.Aug 6 2015, 3:50 PM
demon added a subscriber: demon.

Based on fluorine, these seem to have stopped on the 6th...

demon claimed this task.
demon moved this task from Dec2019/1.35.wmf.10+ to Resolved on the Wikimedia-production-error board.
mmodell removed demon as the assignee of this task.

Fatal error: Class undefined: ReplyAction in /srv/mediawiki/php-1.27.0-wmf.17/includes/actions/Action.php on line 99

I found out that this happens consistently if you add ?action=reply to the URL for a non-Flow page, e.g. https://www.mediawiki.org/wiki/Main_Page?action=reply . However, I think this is a bug in core. I'll write up a patch to fix the symptom in a minute, but here's a rant about the larger problem.

MW core exposes a $wgActions array, which maps action names to classes. An action can also map to true. The documentation says is that $wgAction['reply'] = true; is shorthand for $wgActions['reply'] = 'ReplyAction';. That seems reasonable.

ContentHandler exposes a getActionOverrides() method, which can return an array mapping actions to classes. This allows for nifty things like pointing action=edit to a different class if the title has a certain content handler. This is also great.

However, the logic in Action::getClass() only considers the action overrides from ContentHandler if the entry in $wgActions is set to true. This causes several problems:

  • If MW core defines an action with an explicit class (currently this is only true for editchangetags and revisiondelete), those actions cannot be overridden by a content handler. This seems undesirable and unintuitive. The documentation technically does say this, but it's not clear.
  • If you are an extension registering an action that needs to have content model-specific behavior, you can't decide on the name of the class that does the default handing for the action, it has to be FooAction. This means you can't put this class in the same namespace as the rest of your extension, for example.
  • If you are an extension registering an action that only makes sense for your own content model, you have to create FooAction in the global namespace to prevent MW from crashing.

I think all of these things are terrible. I'll write a patch to make the latter show an "Unknown action" error message instead of crashing, but the extensibility of this whole system is poor. There's no way for Flow to register action=reply in a way that makes sense, and it's not even guaranteed that it can override action=view for its content model because someone could change core in a way that prevents this without realizing that that's what they're doing.

Change 277932 had a related patch set uploaded (by Catrope):
Action:🏭 Don't crash on missing Action classes

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

Change 277932 merged by jenkins-bot:
Action:🏭 Don't crash on missing Action classes

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

demon claimed this task.