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
This commit is contained in:
Brad Jorsch 2014-03-06 09:37:26 -05:00 committed by Ori.livneh
parent ceccc15884
commit 1d79bd6036
2 changed files with 89 additions and 37 deletions

27
api.php
View file

@ -70,10 +70,21 @@ $wgTitle = Title::makeTitle( NS_MAIN, 'API' );
$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 @@ if ( $wgAPIRequestLog ) {
$_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" );

View file

@ -363,37 +363,7 @@ class ApiMain extends ApiBase {
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 @@ class ApiMain extends ApiBase {
ob_end_flush();
}
/**
* 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.
*