From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Tue, 21 Aug 2018 15:19:15 +0000 (-0400)
Subject: API: Catch Errors as well as Exceptions
X-Git-Tag: 1.34.0-rc.0~4326^2
X-Git-Url: http://git.heureux-cyclage.org/?a=commitdiff_plain;h=e6a75d806b91f29573c0038d1d64cd64dcbe09a6;p=lhc%2Fweb%2Fwiklou.git

API: Catch Errors as well as Exceptions

ApiMain (and also api.php) tries to catch any Exception so as to provide
a properly-formatted error message to the client instead of an HTML
error page.

With PHP 7.0, some cases that produce an Exception in HHVM instead
produce an Error. The API code should catch these too.

Fortunately neither Zend PHP nor HHVM care if you try to catch a class
that doesn't exist, so we can just add catch blocks for Throwable and
not worry about it.

Bug: T202416
Change-Id: I189eee466bd09870bc172f2420be393a7c0b1900
---

diff --git a/api.php b/api.php
index 9c5ac95716..9cf75787ba 100644
--- a/api.php
+++ b/api.php
@@ -72,7 +72,11 @@ try {
 	if ( !$processor instanceof ApiMain ) {
 		throw new MWException( 'ApiBeforeMain hook set $processor to a non-ApiMain class' );
 	}
-} catch ( Exception $e ) {
+} catch ( Exception $e ) { // @todo Remove this block when HHVM is no longer supported
+	// Crap. Try to report the exception in API format to be friendly to clients.
+	ApiMain::handleApiBeforeMainException( $e );
+	$processor = false;
+} catch ( Throwable $e ) {
 	// Crap. Try to report the exception in API format to be friendly to clients.
 	ApiMain::handleApiBeforeMainException( $e );
 	$processor = false;
@@ -99,7 +103,9 @@ if ( $wgAPIRequestLog ) {
 		try {
 			$manager = $processor->getModuleManager();
 			$module = $manager->getModule( $wgRequest->getVal( 'action' ), 'action' );
-		} catch ( Exception $ex ) {
+		} catch ( Exception $ex ) { // @todo Remove this block when HHVM is no longer supported
+			$module = null;
+		} catch ( Throwable $ex ) {
 			$module = null;
 		}
 		if ( !$module || $module->mustBePosted() ) {
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 03d29524a8..3b305f9e5c 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -534,7 +534,11 @@ class ApiMain extends ApiBase {
 			MediaWikiServices::getInstance()->getStatsdDataFactory()->timing(
 				'api.' . $this->mModule->getModuleName() . '.executeTiming', 1000 * $runTime
 			);
-		} catch ( Exception $e ) {
+		} catch ( Exception $e ) { // @todo Remove this block when HHVM is no longer supported
+			$this->handleException( $e );
+			$this->logRequest( microtime( true ) - $t, $e );
+			$isError = true;
+		} catch ( Throwable $e ) {
 			$this->handleException( $e );
 			$this->logRequest( microtime( true ) - $t, $e );
 			$isError = true;
@@ -558,9 +562,9 @@ class ApiMain extends ApiBase {
 	 * Handle an exception as an API response
 	 *
 	 * @since 1.23
-	 * @param Exception $e
+	 * @param Exception|Throwable $e
 	 */
-	protected function handleException( Exception $e ) {
+	protected function handleException( $e ) {
 		// T65145: Rollback any open database transactions
 		if ( !( $e instanceof ApiUsageException || $e instanceof UsageException ) ) {
 			// UsageExceptions are intentional, so don't rollback if that's the case
@@ -600,7 +604,10 @@ class ApiMain extends ApiBase {
 			foreach ( $ex->getStatusValue()->getErrors() as $error ) {
 				try {
 					$this->mPrinter->addWarning( $error );
-				} catch ( Exception $ex2 ) {
+				} catch ( Exception $ex2 ) { // @todo Remove this block when HHVM is no longer supported
+					// WTF?
+					$this->addWarning( $error );
+				} catch ( Throwable $ex2 ) {
 					// WTF?
 					$this->addWarning( $error );
 				}
@@ -631,17 +638,20 @@ class ApiMain extends ApiBase {
 	 * friendly to clients. If it fails, it will rethrow the exception.
 	 *
 	 * @since 1.23
-	 * @param Exception $e
-	 * @throws Exception
+	 * @param Exception|Throwable $e
+	 * @throws Exception|Throwable
 	 */
-	public static function handleApiBeforeMainException( Exception $e ) {
+	public static function handleApiBeforeMainException( $e ) {
 		ob_start();
 
 		try {
 			$main = new self( RequestContext::getMain(), false );
 			$main->handleException( $e );
 			$main->logRequest( 0, $e );
-		} catch ( Exception $e2 ) {
+		} catch ( Exception $e2 ) { // @todo Remove this block when HHVM is no longer supported
+			// Nope, even that didn't work. Punt.
+			throw $e;
+		} catch ( Throwable $e2 ) {
 			// Nope, even that didn't work. Punt.
 			throw $e;
 		}
@@ -1009,7 +1019,7 @@ class ApiMain extends ApiBase {
 	 * text around the exception's (presumably English) message as a single
 	 * error (no warnings).
 	 *
-	 * @param Exception $e
+	 * @param Exception|Throwable $e
 	 * @param string $type 'error' or 'warning'
 	 * @return ApiMessage[]
 	 * @since 1.27
@@ -1054,7 +1064,7 @@ class ApiMain extends ApiBase {
 
 	/**
 	 * Replace the result data with the information about an exception.
-	 * @param Exception $e
+	 * @param Exception|Throwable $e
 	 * @return string[] Error codes
 	 */
 	protected function substituteResultWithError( $e ) {
@@ -1609,7 +1619,7 @@ class ApiMain extends ApiBase {
 	/**
 	 * Log the preceding request
 	 * @param float $time Time in seconds
-	 * @param Exception|null $e Exception caught while processing the request
+	 * @param Exception|Throwable|null $e Exception caught while processing the request
 	 */
 	protected function logRequest( $time, $e = null ) {
 		$request = $this->getRequest();