From 362c220aa1bb831d78f99e209bfa218d49f9c9d2 Mon Sep 17 00:00:00 2001
From: Bryan Davis <bd808@wikimedia.org>
Date: Thu, 18 Feb 2016 14:18:14 -0700
Subject: [PATCH] Add request error state to ApiBase::logRequest

Add a boolean indication of the error status of an API request and
a list of error codes to the log event emitted to the 'ApiRequest' debug
log channel.

Bug: T113672
Change-Id: I694a3000bab0d569dfe41d711f05121b917e6104
---
 includes/api/ApiMain.php | 59 +++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 16 deletions(-)

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index e9d6316510..3bdc338c13 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -424,17 +424,17 @@ class ApiMain extends ApiBase {
 		ob_start();
 
 		$t = microtime( true );
+		$isError = false;
 		try {
 			$this->executeAction();
-			$isError = false;
+			$this->logRequest( microtime( true ) - $t );
+
 		} catch ( Exception $e ) {
 			$this->handleException( $e );
+			$this->logRequest( microtime( true ) - $t, $e );
 			$isError = true;
 		}
 
-		// Log the request whether or not there was an error
-		$this->logRequest( microtime( true ) - $t );
-
 		// Commit DBs and send any related cookies and headers
 		MediaWiki::preOutputCommit( $this->getContext() );
 
@@ -530,13 +530,13 @@ class ApiMain extends ApiBase {
 		try {
 			$main = new self( RequestContext::getMain(), false );
 			$main->handleException( $e );
+			$main->logRequest( 0, $e );
 		} catch ( Exception $e2 ) {
 			// Nope, even that didn't work. Punt.
 			throw $e;
 		}
 
-		// Log the request and reset cache headers
-		$main->logRequest( 0 );
+		// Reset cache headers
 		$main->sendCacheHeaders( true );
 
 		ob_end_flush();
@@ -847,20 +847,19 @@ class ApiMain extends ApiBase {
 	}
 
 	/**
-	 * Replace the result data with the information about an exception.
-	 * Returns the error code
+	 * Create an error message for the given exception.
+	 *
+	 * If the exception is a UsageException then
+	 * UsageException::getMessageArray() will be called to create the message.
+	 *
 	 * @param Exception $e
-	 * @return string
+	 * @return array ['code' => 'some string', 'info' => 'some other string']
+	 * @since 1.27
 	 */
-	protected function substituteResultWithError( $e ) {
-		$result = $this->getResult();
-		$config = $this->getConfig();
-
+	protected function errorMessageFromException( $e ) {
 		if ( $e instanceof UsageException ) {
 			// User entered incorrect parameters - generate error response
 			$errMessage = $e->getMessageArray();
-			$link = wfExpandUrl( wfScript( 'api' ) );
-			ApiResult::setContentValue( $errMessage, 'docref', "See $link for API usage" );
 		} else {
 			// Something is seriously wrong
 			if ( ( $e instanceof DBQueryError ) && !$config->get( 'ShowSQLErrors' ) ) {
@@ -873,6 +872,27 @@ class ApiMain extends ApiBase {
 				'code' => 'internal_api_error_' . get_class( $e ),
 				'info' => '[' . MWExceptionHandler::getLogId( $e ) . '] ' . $info,
 			];
+		}
+		return $errMessage;
+	}
+
+	/**
+	 * Replace the result data with the information about an exception.
+	 * Returns the error code
+	 * @param Exception $e
+	 * @return string
+	 */
+	protected function substituteResultWithError( $e ) {
+		$result = $this->getResult();
+		$config = $this->getConfig();
+
+		$errMessage = $this->errorMessageFromException( $e );
+		if ( $e instanceof UsageException ) {
+			// User entered incorrect parameters - generate error response
+			$link = wfExpandUrl( wfScript( 'api' ) );
+			ApiResult::setContentValue( $errMessage, 'docref', "See $link for API usage" );
+		} else {
+			// Something is seriously wrong
 			if ( $config->get( 'ShowExceptionDetails' ) ) {
 				ApiResult::setContentValue(
 					$errMessage,
@@ -1337,8 +1357,9 @@ class ApiMain extends ApiBase {
 	/**
 	 * Log the preceding request
 	 * @param float $time Time in seconds
+	 * @param Exception $e Exception caught while processing the request
 	 */
-	protected function logRequest( $time ) {
+	protected function logRequest( $time, $e = null ) {
 		$request = $this->getRequest();
 		$logCtx = [
 			'ts' => time(),
@@ -1346,9 +1367,15 @@ class ApiMain extends ApiBase {
 			'userAgent' => $this->getUserAgent(),
 			'wiki' => wfWikiID(),
 			'timeSpentBackend' => round( $time * 1000 ),
+			'hadError' => $e !== null,
+			'errorCodes' => [],
 			'params' => [],
 		];
 
+		if ( $e ) {
+			$logCtx['errorCodes'][] = $this->errorMessageFromException( $e )['code'];
+		}
+
 		// Construct space separated message for 'api' log channel
 		$msg = "API {$request->getMethod()} " .
 			wfUrlencode( str_replace( ' ', '_', $this->getUser()->getName() ) ) .
-- 
2.20.1