From 8b7f9129c390ff9e536ca0f95c52573d46af381a Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Wed, 11 Jun 2025 17:12:51 -0700 Subject: [PATCH] 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) --- includes/Setup.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/Setup.php b/includes/Setup.php index f3ddda9bcb7..8e819fd42d0 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -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 PSR-3 logging ' . + 'Error: MediaWiki requires the PSR-3 logging ' . "library 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 instructions for installing libraries 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.