Anomie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/117189
Change subject: API: Handle exceptions from ApiBeforeMain hook in a
user-friendly manner
......................................................................
API: Handle exceptions from ApiBeforeMain hook in a user-friendly manner
The immediate impetus behind this change is this series of events:
1. CirrusSearch hooks ApiBeforeMain to handle some setup that requires
the User object.
2. So User is loaded from the session.
3. OAuth checks the headers as part of loading User.
4. OAuth sees that the headers are invalid, and since it was called from
the API it throws a UsageException, expecting the API to catch it and
return an appropriate response to the client.
5. But nothing does so, leading to an unhelpful "Internal Error" page
being returned to the client.
We can do better than that.
Bug: 62312
Change-Id: Ib5735661eec6ebe57eaa69c67b399e703cc90fc4
---
M api.php
M includes/api/ApiMain.php
2 files changed, 89 insertions(+), 37 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/89/117189/1
diff --git a/api.php b/api.php
index 0d2312a..554c272 100644
--- a/api.php
+++ b/api.php
@@ -70,10 +70,21 @@
$processor = new ApiMain( RequestContext::getMain(), $wgEnableWriteAPI );
// Last chance hook before executing the API
-wfRunHooks( 'ApiBeforeMain', array( &$processor ) );
+try {
+ wfRunHooks( 'ApiBeforeMain', array( &$processor ) );
+ if ( !$processor instanceof ApiMain ) {
+ throw new MWException( 'ApiBeforMain hook set $processor to a
non-ApiMain class' );
+ }
+} catch ( Exception $e ) {
+ // Crap. Try to report the exception in API format to be friendly to
clients.
+ ApiMain::handleApiBeforeMainException( $e );
+ $processor = false;
+}
// Process data & print results
-$processor->execute();
+if ( $processor ) {
+ $processor->execute();
+}
if ( function_exists( 'fastcgi_finish_request' ) ) {
fastcgi_finish_request();
@@ -97,11 +108,15 @@
$_SERVER['HTTP_USER_AGENT']
);
$items[] = $wgRequest->wasPosted() ? 'POST' : 'GET';
- $module = $processor->getModule();
- if ( $module->mustBePosted() ) {
- $items[] = "action=" . $wgRequest->getVal( 'action' );
+ if ( $processor ) {
+ $module = $processor->getModule();
+ if ( $module->mustBePosted() ) {
+ $items[] = "action=" . $wgRequest->getVal( 'action' );
+ } else {
+ $items[] = wfArrayToCgi( $wgRequest->getValues() );
+ }
} else {
- $items[] = wfArrayToCgi( $wgRequest->getValues() );
+ $items[] = "failed in ApiBeforeMain";
}
wfErrorLog( implode( ',', $items ) . "\n", $wgAPIRequestLog );
wfDebug( "Logged API request to $wgAPIRequestLog\n" );
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 0939dea..fea002e 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -362,37 +362,7 @@
try {
$this->executeAction();
} catch ( Exception $e ) {
- // Allow extra cleanup and logging
- wfRunHooks( 'ApiMain::onException', array( $this, $e )
);
-
- // Log it
- if ( !( $e instanceof UsageException ) ) {
- MWExceptionHandler::logException( $e );
- }
-
- // Handle any kind of exception by outputting properly
formatted error message.
- // If this fails, an unhandled exception should be
thrown so that global error
- // handler will process and log it.
-
- $errCode = $this->substituteResultWithError( $e );
-
- // Error results should not be cached
- $this->setCacheMode( 'private' );
-
- $response = $this->getRequest()->response();
- $headerStr = 'MediaWiki-API-Error: ' . $errCode;
- if ( $e->getCode() === 0 ) {
- $response->header( $headerStr );
- } else {
- $response->header( $headerStr, true,
$e->getCode() );
- }
-
- // Reset and print just the error message
- ob_clean();
-
- // If the error occurred during printing, do a
printer->profileOut()
- $this->mPrinter->safeProfileOut();
- $this->printResult( true );
+ $this->handleException( $e );
}
// Log the request whether or not there was an error
@@ -410,6 +380,73 @@
}
/**
+ * Handle an exception as an API response
+ *
+ * @since 1.23
+ * @param Exception $e
+ */
+ protected function handleException( Exception $e ) {
+ // Allow extra cleanup and logging
+ wfRunHooks( 'ApiMain::onException', array( $this, $e ) );
+
+ // Log it
+ if ( !( $e instanceof UsageException ) ) {
+ MWExceptionHandler::logException( $e );
+ }
+
+ // Handle any kind of exception by outputting properly
formatted error message.
+ // If this fails, an unhandled exception should be thrown so
that global error
+ // handler will process and log it.
+
+ $errCode = $this->substituteResultWithError( $e );
+
+ // Error results should not be cached
+ $this->setCacheMode( 'private' );
+
+ $response = $this->getRequest()->response();
+ $headerStr = 'MediaWiki-API-Error: ' . $errCode;
+ if ( $e->getCode() === 0 ) {
+ $response->header( $headerStr );
+ } else {
+ $response->header( $headerStr, true, $e->getCode() );
+ }
+
+ // Reset and print just the error message
+ ob_clean();
+
+ // If the error occurred during printing, do a
printer->profileOut()
+ $this->mPrinter->safeProfileOut();
+ $this->printResult( true );
+ }
+
+ /**
+ * Handle an exception from the ApiBeforeMain hook.
+ *
+ * This tries to print the exception as an API response, to be more
+ * friendly to clients. If it fails, it will rethrow the exception.
+ *
+ * @since 1.23
+ * @param Exception $e
+ */
+ public static function handleApiBeforeMainException( Exception $e ) {
+ ob_start();
+
+ try {
+ $main = new self( RequestContext::getMain(), false );
+ $main->handleException( $e );
+ } catch ( Exception $e2 ) {
+ // Nope, even that didn't work. Punt.
+ throw $e;
+ }
+
+ // Log the request and reset cache headers
+ $main->logRequest( 0 );
+ $main->sendCacheHeaders();
+
+ ob_end_flush();
+ }
+
+ /**
* Check the &origin= query parameter against the Origin: HTTP header
and respond appropriately.
*
* If no origin parameter is present, nothing happens.
--
To view, visit https://gerrit.wikimedia.org/r/117189
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib5735661eec6ebe57eaa69c67b399e703cc90fc4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
