From 87cd44a9c536974cd7a64666faddfd5aa625f6e6 Mon Sep 17 00:00:00 2001
From: daniel <dkinzler@wikimedia.org>
Date: Tue, 19 Mar 2019 23:16:49 +0100
Subject: [PATCH] Stop gap to shut up log spam due to T212428.

Bug: T212428
Change-Id: I442bbe9837e73167cfdaabb0451bf974dc5039f3
---
 includes/api/ApiQueryRevisionsBase.php | 150 +++++++++++++++----------
 1 file changed, 91 insertions(+), 59 deletions(-)

diff --git a/includes/api/ApiQueryRevisionsBase.php b/includes/api/ApiQueryRevisionsBase.php
index 51f4d41299..565e615edd 100644
--- a/includes/api/ApiQueryRevisionsBase.php
+++ b/includes/api/ApiQueryRevisionsBase.php
@@ -20,6 +20,7 @@
  * @file
  */
 
+use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\Revision\RevisionAccessException;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Revision\SlotRecord;
@@ -292,69 +293,27 @@ abstract class ApiQueryRevisionsBase extends ApiQueryGeneratorBase {
 			}
 		}
 
-		if ( $this->fld_roles ) {
-			$vals['roles'] = $revision->getSlotRoles();
-		}
-
-		if ( $this->needSlots ) {
-			$revDel = $this->checkRevDel( $revision, RevisionRecord::DELETED_TEXT );
-			if ( ( $this->fld_slotsha1 || $this->fetchContent ) && ( $revDel & self::IS_DELETED ) ) {
-				$anyHidden = true;
+		try {
+			if ( $this->fld_roles ) {
+				$vals['roles'] = $revision->getSlotRoles();
 			}
-			if ( $this->slotRoles === null ) {
-				try {
-					$slot = $revision->getSlot( SlotRecord::MAIN, RevisionRecord::RAW );
-				} catch ( RevisionAccessException $e ) {
-					// Back compat: If there's no slot, there's no content, so set 'textmissing'
-					// @todo: Gergő says to mention T198099 as a "todo" here.
-					$vals['textmissing'] = true;
-					$slot = null;
-				}
 
-				if ( $slot ) {
-					$content = null;
-					$vals += $this->extractSlotInfo( $slot, $revDel, $content );
-					if ( !empty( $vals['nosuchsection'] ) ) {
-						$this->dieWithError(
-							[
-								'apierror-nosuchsection-what',
-								wfEscapeWikiText( $this->section ),
-								$this->msg( 'revid', $revision->getId() )
-							],
-							'nosuchsection'
-						);
-					}
-					if ( $content ) {
-						$vals += $this->extractDeprecatedContent( $content, $revision );
-					}
-				}
-			} else {
-				$roles = array_intersect( $this->slotRoles, $revision->getSlotRoles() );
-				$vals['slots'] = [
-					ApiResult::META_KVP_MERGE => true,
-				];
-				foreach ( $roles as $role ) {
-					try {
-						$slot = $revision->getSlot( $role, RevisionRecord::RAW );
-					} catch ( RevisionAccessException $e ) {
-						// Don't error out here so the client can still process other slots/revisions.
-						// @todo: Gergő says to mention T198099 as a "todo" here.
-						$vals['slots'][$role]['missing'] = true;
-						continue;
-					}
-					$content = null;
-					$vals['slots'][$role] = $this->extractSlotInfo( $slot, $revDel, $content );
-					// @todo Move this into extractSlotInfo() (and remove its $content parameter)
-					// when extractDeprecatedContent() is no more.
-					if ( $content ) {
-						$vals['slots'][$role]['contentmodel'] = $content->getModel();
-						$vals['slots'][$role]['contentformat'] = $content->getDefaultFormat();
-						ApiResult::setContentValue( $vals['slots'][$role], 'content', $content->serialize() );
-					}
+			if ( $this->needSlots ) {
+				$revDel = $this->checkRevDel( $revision, RevisionRecord::DELETED_TEXT );
+				if ( ( $this->fld_slotsha1 || $this->fetchContent ) && ( $revDel & self::IS_DELETED ) ) {
+					$anyHidden = true;
 				}
-				ApiResult::setArrayType( $vals['slots'], 'kvp', 'role' );
-				ApiResult::setIndexedTagName( $vals['slots'], 'slot' );
+				$vals = array_merge( $vals, $this->extractAllSlotInfo( $revision, $revDel ) );
 			}
+		} catch ( RevisionAccessException $ex ) {
+			// This is here so T212428 doesn't spam the log.
+			// TODO: find out why T212428 happens in the first place!
+			$vals['slotsmissing'] = true;
+
+			LoggerFactory::getInstance( 'api-warning' )->error(
+				'Failed to access revision slots',
+				[ 'revision' => $revision->getId(), 'exception' => $ex, ]
+			);
 		}
 
 		if ( $this->fld_comment || $this->fld_parsedcomment ) {
@@ -396,6 +355,79 @@ abstract class ApiQueryRevisionsBase extends ApiQueryGeneratorBase {
 		return $vals;
 	}
 
+	/**
+	 * Extracts information about all relevant slots.
+	 *
+	 * @param RevisionRecord $revision
+	 * @param int $revDel
+	 *
+	 * @return array
+	 * @throws ApiUsageException
+	 */
+	private function extractAllSlotInfo( RevisionRecord $revision, $revDel ): array {
+		$vals = [];
+
+		if ( $this->slotRoles === null ) {
+			try {
+				$slot = $revision->getSlot( SlotRecord::MAIN, RevisionRecord::RAW );
+			} catch ( RevisionAccessException $e ) {
+				// Back compat: If there's no slot, there's no content, so set 'textmissing'
+				// @todo: Gergő says to mention T198099 as a "todo" here.
+				$vals['textmissing'] = true;
+				$slot = null;
+			}
+
+			if ( $slot ) {
+				$content = null;
+				$vals += $this->extractSlotInfo( $slot, $revDel, $content );
+				if ( !empty( $vals['nosuchsection'] ) ) {
+					$this->dieWithError(
+						[
+							'apierror-nosuchsection-what',
+							wfEscapeWikiText( $this->section ),
+							$this->msg( 'revid', $revision->getId() )
+						],
+						'nosuchsection'
+					);
+				}
+				if ( $content ) {
+					$vals += $this->extractDeprecatedContent( $content, $revision );
+				}
+			}
+		} else {
+			$roles = array_intersect( $this->slotRoles, $revision->getSlotRoles() );
+			$vals['slots'] = [
+				ApiResult::META_KVP_MERGE => true,
+			];
+			foreach ( $roles as $role ) {
+				try {
+					$slot = $revision->getSlot( $role, RevisionRecord::RAW );
+				} catch ( RevisionAccessException $e ) {
+					// Don't error out here so the client can still process other slots/revisions.
+					// @todo: Gergő says to mention T198099 as a "todo" here.
+					$vals['slots'][$role]['missing'] = true;
+					continue;
+				}
+				$content = null;
+				$vals['slots'][$role] = $this->extractSlotInfo( $slot, $revDel, $content );
+				// @todo Move this into extractSlotInfo() (and remove its $content parameter)
+				// when extractDeprecatedContent() is no more.
+				if ( $content ) {
+					$vals['slots'][$role]['contentmodel'] = $content->getModel();
+					$vals['slots'][$role]['contentformat'] = $content->getDefaultFormat();
+					ApiResult::setContentValue(
+						$vals['slots'][$role],
+						'content',
+						$content->serialize()
+					);
+				}
+			}
+			ApiResult::setArrayType( $vals['slots'], 'kvp', 'role' );
+			ApiResult::setIndexedTagName( $vals['slots'], 'slot' );
+		}
+		return $vals;
+	}
+
 	/**
 	 * Extract information from the SlotRecord
 	 *
-- 
2.20.1