Stop gap to shut up log spam due to T212428.
authordaniel <dkinzler@wikimedia.org>
Tue, 19 Mar 2019 22:16:49 +0000 (23:16 +0100)
committerTimo Tijhof <krinklemail@gmail.com>
Tue, 26 Mar 2019 01:43:46 +0000 (01:43 +0000)
Bug: T212428
Change-Id: I442bbe9837e73167cfdaabb0451bf974dc5039f3

includes/api/ApiQueryRevisionsBase.php

index 51f4d41..565e615 100644 (file)
@@ -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
         *