Page MenuHomePhabricator

Avoid exit to make MW handle page exceptions properly
Closed, ResolvedPublic

Description

Author: Webbed.Pete

Description:
MediaWiki contains a few calls to "exit" after generating pages that otherwise
would be cleanly generated. Beginning with PHP5, this is no longer necessary as
an error-handling mechanism.

The attached patch replaces all such exit calls with exception 'throw' and adds
a try/catch framework to index.php

This has the beneficial side effect of making MW far more easily customized via
minor adjustments to index.php


Version: 1.10.x
Severity: trivial

Details

Reference
bz9243

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:35 PM
bzimport set Reference to bz9243.
bzimport added a subscriber: Unknown Object (MLST).

Webbed.Pete wrote:

Patch implements try/catch duplicating present function

It is possible that the "catch" should come earlier in index.php -- is it
really desirable to skip all final page-handling when page-generation hits a
snag?

This patch implements the exception as if we really do want to do the
equivalent of "exit" under default conditions... but that may not be correct.

Attached:

ayg wrote:

If exceptions are used, they should presumably be MWExceptions and give descriptive error
messages (although I suppose it doesn't make much difference).

My impression is that those are places where it really _oughtn't_ to be exiting,
but rather things should be allowed to close out gracefully. So rather than
using exceptions of any kind, I'd rather see them changed to just exit (this may
require fixing code that uses them that makes the assumption they cause exits).

Webbed.Pete wrote:

Brian, I presume you mean "changed to just return". Yes, the whole issue is how
to close out gracefully after giving a "nice" wikitext error message to the
user, an error that has stopped normal page processing in its tracks (e.g. "no
permission".)

It would be great if someone with more MW code experience is able to discern an
appropriate return-based strategy that doesn't cause hiccups to the entire code
base. Any takers?

In the meantime, I'm happy with exception-throwing as an interim improvement
over the current exit "strategy" :)

If this bugfix is to use MWExceptions, we would a) need a new type of
MWException (producing no report); and b) be assured that catching MWExceptions
at the outer level is not an issue.

As for descriptive error messages, sure, that can be done. Simetrical is correct
that it doesn't make much difference -- the message would be invisible to the
user, and solely for developer understanding. In these cases, the code has
already created informative wikitext error messages for the user; there's
nothing more to be created for the user.

The sole purpose of this change is to enable proper cleanup. In my case, all of
MW is wrapped inside another package, so "proper cleanup" has rather large
significance :)

Done in r45314, except for the index.php stuff

those bits were changed a bunch in r45320