Merge "Add request error state to ApiBase::logRequest"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Mon, 29 Feb 2016 16:49:00 +0000 (16:49 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 29 Feb 2016 16:49:00 +0000 (16:49 +0000)
1  2 
includes/api/ApiMain.php

diff --combined includes/api/ApiMain.php
@@@ -424,17 -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() );
  
                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();
        }
  
        /**
-        * 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' ) ) {
                                '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,
                if ( $this->getConfig()->get( 'ShowHostnames' ) ) {
                        $servedby = $this->getParameter( 'servedby' );
                        if ( $servedby ) {
 -                              $result->addValue( null, 'servedby', wfHostName() );
 +                              $result->addValue( null, 'servedby', wfHostname() );
                        }
                }
  
        /**
         * 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(),
                        '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() ) ) .