Setup: Switch vendor error from echo+E_USER_ERROR to echo+exit
== Background ==
E_USER_ERROR is a deprecated error code for "recoverable fatal error",
a confusing description no longer used upstream and replaced by the
Throwable/Error concept, i.e. something that is meant to be fatal,
but could in theory be caught if you know what you're doing, via a
risky catch for Throwable instead of Exception.
What trigger_error with E_USER_ERROR does:
* (If we haven't sent headers yet)
Emit header "HTTP/1.1 500 Internal Server Error".
* (If display_errors is enabled)
Print the message, again, along with a strack trace.
* Notify set_error_handler letting you "catch" a non-Exception error.
* Write it to error_log, e.g. STDERR for composer serve and CLI,
or an Apache/php-fpm error.log file.
* End with exit(1).
Issues:
* When enabling display_errors, the message is printed twice.
* The HTTP 500 status didn't work because headers are already sent,
... by the "echo" statement, right above it.
== Option A: throw Error $message ==
`throw Error($message)` is the natural successor to E_USER_ERROR.
I would recommend this, if
1) we didn't already echo it, and
2) the message didn't contain HTML, and
3) we needed to keep compat with someone catching this, or
4) we wanted a stack trace.
We echo it because display_errors can be off, and the most likely
audience for this is someone new to PHP/MediaWiki, installing in prod
or locally, when debugging is either intentionally off, or before
they're familiar with debugging modes. As such, we want to print it
ourselves either way, and printing it again as part of E_USER_ERROR
isn't needed.
The HTML part is important because one subtle difference between
trigger_error and throw Error is that the former allows raw HTML,
while the latter treats exception messages as plain text. Our message
intentionally uses HTML to link to docs in the browser, so this is
unhelpful.
The catchable-ness of this is not important to us, as no extension or
distro code (e.g. PlatformSettings.php) can run this early. There
are no runtime consumers of this error, only the end-user's browser.
== Option B: echo+exit ==
Given we already print the message, we just need to exit.
== History ==
* 2014 (Ie66794441): Add first ever Composer dependency (psr/log)
* 2015 (Ie47467657): Add LoggerFactory with check for missing Composer
dependency, to address a then-common issue.
* 2015 (Ib60261237): Move check earlier, to WebStart.
* 2017 (I633a6ff23): Move check earlier, to Setup.
* 2021 (Ia81903fb2): Remove redundant exit(1).
== Change ==
* Emit HTTP 500 before the echo.
* Keep echo (for browser) and error_log (for discovery via CLI or log
file).
* Replace trigger_error with just exit(1), avoid duplicate message.
Bug: T379445
Change-Id: I6050ec4ca857d3c92c1c43f6a38e4154cd60e5d5
(cherry picked from commit 98c6d3c4c3511ecf60ffc693ff6c7164964270ca)
This commit is contained in:
parent
65ca36bb1d
commit
8b7f9129c3
1 changed files with 4 additions and 2 deletions
|
|
@ -129,15 +129,17 @@ require_once MW_INSTALL_PATH . '/includes/Defines.php';
|
|||
// Assert that composer dependencies were successfully loaded
|
||||
if ( !interface_exists( LoggerInterface::class ) ) {
|
||||
$message = (
|
||||
'MediaWiki requires the <a href="https://github.com/php-fig/log">PSR-3 logging ' .
|
||||
'<strong>Error:</strong> MediaWiki requires the <a href="https://github.com/php-fig/log">PSR-3 logging ' .
|
||||
"library</a> to be present. This library is not embedded directly in MediaWiki's " .
|
||||
"git repository and must be installed separately by the end user.\n\n" .
|
||||
'Please see the <a href="https://www.mediawiki.org/wiki/Download_from_Git' .
|
||||
'#Fetch_external_libraries">instructions for installing libraries</a> on mediawiki.org ' .
|
||||
'for help on installing the required components.'
|
||||
);
|
||||
http_response_code( 500 );
|
||||
echo $message;
|
||||
trigger_error( $message, E_USER_ERROR );
|
||||
error_log( $message );
|
||||
exit( 1 );
|
||||
}
|
||||
|
||||
// Deprecated global variable for backwards-compatibility.
|
||||
|
|
|
|||
Loading…
Reference in a new issue