Gergő Tisza has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/371584 )
Change subject: Do not circumvent ExceptionRenderer for MWExceptions
......................................................................
Do not circumvent ExceptionRenderer for MWExceptions
* allow ExceptionRenderer to override how MWExceptions are displayed
* remove duplicate code from MWException
* use API/commandline-specific code (it was overridden in some
subclasses)
Task: T111731
Change-Id: If3747eab5fb3499708956fd8282a88ca973b4bc4
---
M includes/exception/BadRequestError.php
M includes/exception/HttpError.php
M includes/exception/MWException.php
M includes/exception/MWExceptionHandler.php
M includes/exception/Renderer.php
M includes/exception/StandardRenderer.php
M includes/exception/ThrottledError.php
M includes/filerepo/file/LocalFile.php
M tests/phpunit/includes/exception/MWExceptionTest.php
9 files changed, 49 insertions(+), 79 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/84/371584/1
diff --git a/includes/exception/BadRequestError.php
b/includes/exception/BadRequestError.php
index 5fcf0e6..a125242 100644
--- a/includes/exception/BadRequestError.php
+++ b/includes/exception/BadRequestError.php
@@ -26,9 +26,7 @@
*/
class BadRequestError extends ErrorPageError {
- public function report() {
- global $wgOut;
- $wgOut->setStatusCode( 400 );
- parent::report();
+ public function getStatusCode() {
+ return 400;
}
}
diff --git a/includes/exception/HttpError.php b/includes/exception/HttpError.php
index f464d8a..3f2f02d 100644
--- a/includes/exception/HttpError.php
+++ b/includes/exception/HttpError.php
@@ -72,9 +72,6 @@
public function report() {
$this->doLog();
- HttpStatus::header( $this->httpCode );
- header( 'Content-type: text/html; charset=utf-8' );
-
print $this->getHTML();
}
diff --git a/includes/exception/MWException.php
b/includes/exception/MWException.php
index 8c1f8dc..ab11775 100644
--- a/includes/exception/MWException.php
+++ b/includes/exception/MWException.php
@@ -146,7 +146,17 @@
}
/**
+ * Return the HTTP status code appropriate for this exception.
+ *
+ * @return int
+ */
+ public function getStatusCode() {
+ return 500;
+ }
+
+ /**
* Output the exception report using HTML.
+ * @deprecated since 1.30, create an ExceptionRenderer instead
*/
public function reportHTML() {
global $wgOut, $wgSitename;
@@ -176,38 +186,10 @@
/**
* Output a report about the exception and takes care of formatting.
* It will be either HTML or plain text based on isCommandLine().
+ * @deprecated since 1.30, create an ExceptionRenderer instead
*/
public function report() {
- global $wgMimeType;
-
- if ( defined( 'MW_API' ) ) {
- // Unhandled API exception, we can't be sure that
format printer is alive
- self::header( 'MediaWiki-API-Error:
internal_api_error_' . static::class );
- wfHttpError( 500, 'Internal Server Error',
$this->getText() );
- } elseif ( self::isCommandLine() ) {
- $message = $this->getText();
- // T17602: STDERR may not be available
- if ( defined( 'STDERR' ) ) {
- fwrite( STDERR, $message );
- } else {
- echo $message;
- }
- } else {
- self::statusHeader( 500 );
- self::header( "Content-Type: $wgMimeType;
charset=utf-8" );
-
- $this->reportHTML();
- }
- }
-
- /**
- * Check whether we are in command line mode or not to report the
exception
- * in the correct format.
- *
- * @return bool
- */
- public static function isCommandLine() {
- return !empty( $GLOBALS['wgCommandLineMode'] );
+ $this->reportHTML();
}
/**
@@ -218,11 +200,6 @@
private static function header( $header ) {
if ( !headers_sent() ) {
header( $header );
- }
- }
- private static function statusHeader( $code ) {
- if ( !headers_sent() ) {
- HttpStatus::header( $code );
}
}
}
diff --git a/includes/exception/MWExceptionHandler.php
b/includes/exception/MWExceptionHandler.php
index 2bbea0c..5a06208 100644
--- a/includes/exception/MWExceptionHandler.php
+++ b/includes/exception/MWExceptionHandler.php
@@ -68,14 +68,7 @@
$renderer =
MediaWikiServices::getInstance()->getExceptionRenderer();
try {
// Try and show the exception prettily, with the normal
skin infrastructure
- if ( $e instanceof MWException ) {
- // Delegate to MWException until all subclasses
are handled by
- // MWExceptionRenderer and
MWException::report() has been
- // removed.
- $e->report();
- } else {
- $renderer->output( $e, Renderer::AS_PRETTY );
- }
+ $renderer->output( $e, Renderer::AS_PRETTY );
} catch ( Exception $e2 ) {
// Exception occurred from within exception handler
// Show a simpler message for the original exception,
diff --git a/includes/exception/Renderer.php b/includes/exception/Renderer.php
index ae57bcb..6d9a3b3 100644
--- a/includes/exception/Renderer.php
+++ b/includes/exception/Renderer.php
@@ -95,6 +95,19 @@
}
/**
+ * Return the HTTP status code appropriate for this exception.
+ * @param Exception|Throwable $e
+ * @return int
+ */
+ protected function getStatusCode( $e ) {
+ if ( $e instanceof MWException ) {
+ return $e->getStatusCode();
+ } else {
+ return 500;
+ }
+ }
+
+ /**
* Add a status header, but only if it is safe to do.
* @param integer $code
*/
diff --git a/includes/exception/StandardRenderer.php
b/includes/exception/StandardRenderer.php
index 873358f..2a92a09 100644
--- a/includes/exception/StandardRenderer.php
+++ b/includes/exception/StandardRenderer.php
@@ -24,6 +24,7 @@
use Exception;
use Html;
use MessageCache;
+use MWException;
use MWExceptionHandler;
use Throwable;
use WebRequest;
@@ -43,6 +44,10 @@
* to show that information.
*/
public function getHTML( $e ) {
+ if ( $e instanceof MWException ) {
+ return $e->getHTML();
+ }
+
if ( $this->showBackTrace( $e ) ) {
$html = "<div class=\"errorbox mw-content-ltr\"><p>" .
nl2br( htmlspecialchars(
MWExceptionHandler::getLogMessage( $e ) ) ) .
@@ -70,7 +75,9 @@
* @inheritdoc
*/
public function getText( $e ) {
- if ( $this->showBackTrace( $e ) ) {
+ if ( $e instanceof MWException ) {
+ return $e->getText();
+ } elseif ( $this->showBackTrace( $e ) ) {
return MWExceptionHandler::getLogMessage( $e ) .
"\nBacktrace:\n" .
MWExceptionHandler::getRedactedTraceAsString( $e ) . "\n";
@@ -86,7 +93,7 @@
global $wgMimeType;
if ( $mode === self::AS_PRETTY ) {
- $this->statusHeader( 500 );
+ $this->statusHeader( $this->getStatusCode( $e ) );
if ( $e instanceof DBConnectionError ) {
$this->reportOutageHTML( $e );
} else {
@@ -131,6 +138,14 @@
private function reportHTML( $e ) {
global $wgSitename;
+ if ( $e instanceof MWException ) {
+ // Delegate to MWException until all subclasses are
handled by
+ // MWExceptionRenderer and MWException::report() has
been
+ // removed.
+ $e->reportHTML();
+ return;
+ }
+
$outputPage = $this->getOutputPage( $e );
$pageTitle = $this->getErrorPageTitle( $e );
$customMessage = $this->getCustomMessage( $e );
diff --git a/includes/exception/ThrottledError.php
b/includes/exception/ThrottledError.php
index bec0d90..69165fc 100644
--- a/includes/exception/ThrottledError.php
+++ b/includes/exception/ThrottledError.php
@@ -32,9 +32,7 @@
);
}
- public function report() {
- global $wgOut;
- $wgOut->setStatusCode( 429 );
- parent::report();
+ public function getStatusCode() {
+ return 429;
}
}
diff --git a/includes/filerepo/file/LocalFile.php
b/includes/filerepo/file/LocalFile.php
index 33177d3..6340dcb 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -3187,9 +3187,7 @@
);
}
- public function report() {
- global $wgOut;
- $wgOut->setStatusCode( 429 );
- parent::report();
+ public function getStatusCode() {
+ return 429;
}
}
diff --git a/tests/phpunit/includes/exception/MWExceptionTest.php
b/tests/phpunit/includes/exception/MWExceptionTest.php
index 614a1c9..7e6f4f4 100644
--- a/tests/phpunit/includes/exception/MWExceptionTest.php
+++ b/tests/phpunit/includes/exception/MWExceptionTest.php
@@ -76,25 +76,6 @@
}
/**
- * @dataProvider provideIsCommandLine
- * @covers MWException::isCommandLine
- */
- public function testisCommandLine( $expected, $wgCommandLineMode ) {
- $this->setMwGlobals( [
- 'wgCommandLineMode' => $wgCommandLineMode,
- ] );
- $e = new MWException();
- $this->assertEquals( $expected, $e->isCommandLine() );
- }
-
- public static function provideIsCommandLine() {
- return [
- [ false, null ],
- [ true, true ],
- ];
- }
-
- /**
* Verify the exception classes are JSON serializabe.
*
* @covers MWExceptionHandler::jsonSerializeException
--
To view, visit https://gerrit.wikimedia.org/r/371584
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If3747eab5fb3499708956fd8282a88ca973b4bc4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
