Improve handling of Parsoid resource limit exceeded exceptions

Follow-up to 3d9585acbe.

* Avoid deprecated method StatusValue::getErrors()
* Don't use exceptions for control flow
* Don't re-use Parsoid's ResourceLimitExceededException,
  it seems unexpected to me and might limit future refactoring
* Use dieStatus() instead of dieWithException(), which will produce
  better localised error messages
* Add missing error handling in ApiDiscussionToolsThank

Change-Id: Ia032025f92f1c3cc0d62a0f45925dddba93fb42f

includes/ApiDiscussionToolsCompare.php

@@ -11,7 +11,6 @@ use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Title\Title;
 use Wikimedia\ParamValidator\ParamValidator;
-use Wikimedia\Parsoid\Core\ResourceLimitExceededException;
 
 class ApiDiscussionToolsCompare extends ApiBase {
 
@@ -89,12 +88,16 @@ class ApiDiscussionToolsCompare extends ApiBase {
 			return;
 		}
 
-		try {
-			$fromItemSet = HookUtils::parseRevisionParsoidHtml( $fromRev, __METHOD__ );
-			$toItemSet = HookUtils::parseRevisionParsoidHtml( $toRev, __METHOD__ );
-		} catch ( ResourceLimitExceededException $e ) {
-			$this->dieWithException( $e );
+		$fromStatus = HookUtils::parseRevisionParsoidHtml( $fromRev, __METHOD__ );
+		$toStatus = HookUtils::parseRevisionParsoidHtml( $toRev, __METHOD__ );
+		if ( !$fromStatus->isOK() ) {
+			$this->dieStatus( $fromStatus );
 		}
+		if ( !$toStatus->isOK() ) {
+			$this->dieStatus( $toStatus );
+		}
+		$fromItemSet = $fromStatus->getValueOrThrow();
+		$toItemSet = $toStatus->getValueOrThrow();
 
 		$removedComments = [];
 		foreach ( $fromItemSet->getCommentItems() as $fromComment ) {

includes/ApiDiscussionToolsPageInfo.php

@@ -15,7 +15,6 @@ use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Title\Title;
 use Wikimedia\ParamValidator\ParamValidator;
-use Wikimedia\Parsoid\Core\ResourceLimitExceededException;
 use Wikimedia\Parsoid\DOM\Element;
 use Wikimedia\Parsoid\DOM\Text;
 use Wikimedia\Parsoid\Utils\DOMUtils;
@@ -117,11 +116,11 @@ class ApiDiscussionToolsPageInfo extends ApiBase {
 			$this->dieWithError( [ 'apierror-missingcontent-revid', $revision->getId() ], 'missingcontent' );
 		}
 
-		try {
-			return HookUtils::parseRevisionParsoidHtml( $revision, __METHOD__ );
-		} catch ( ResourceLimitExceededException $e ) {
-			$this->dieWithException( $e );
+		$status = HookUtils::parseRevisionParsoidHtml( $revision, __METHOD__ );
+		if ( !$status->isOK() ) {
+			$this->dieStatus( $status );
 		}
+		return $status->getValueOrThrow();
 	}
 
 	/**

includes/ApiDiscussionToolsThank.php

@@ -16,7 +16,6 @@ use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Title\Title;
 use MediaWiki\User\UserFactory;
 use Wikimedia\ParamValidator\ParamValidator;
-use Wikimedia\Parsoid\Core\ResourceLimitExceededException;
 
 /**
  * API module to send DiscussionTools comment thanks notifications
@@ -49,7 +48,6 @@ class ApiDiscussionToolsThank extends ApiThank {
 	/**
 	 * @inheritDoc
 	 * @throws ApiUsageException
-	 * @throws ResourceLimitExceededException
 	 */
 	public function execute() {
 		$user = $this->getUser();
@@ -77,7 +75,11 @@ class ApiDiscussionToolsThank extends ApiThank {
 				'nosuchrevid'
 			);
 		}
-		$threadItemSet = HookUtils::parseRevisionParsoidHtml( $revision, __METHOD__ );
+		$status = HookUtils::parseRevisionParsoidHtml( $revision, __METHOD__ );
+		if ( !$status->isOK() ) {
+			$this->dieStatus( $status );
+		}
+		$threadItemSet = $status->getValueOrThrow();
 
 		$comment = $threadItemSet->findCommentById( $commentId );
 

includes/ContentThreadItemSetStatus.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace MediaWiki\Extension\DiscussionTools;
+
+use MediaWiki\Status\Status;
+use StatusValue;
+use Wikimedia\NormalizedException\NormalizedException;
+
+class ContentThreadItemSetStatus extends StatusValue {
+
+	/**
+	 * Convenience method.
+	 */
+	public static function wrap( StatusValue $other ): self {
+		return ( new self )->merge( $other, true );
+	}
+
+	/**
+	 * Like getValue(), but will throw if the value is null. Check isOK() first to avoid errors.
+	 */
+	public function getValueOrThrow(): ContentThreadItemSet {
+		$value = $this->getValue();
+		if ( $value === null ) {
+			throw new NormalizedException( ...Status::wrap( $this )->getPsr3MessageAndContext() );
+		}
+		return $value;
+	}
+
+}

includes/Hooks/DataUpdatesHooks.php

@@ -45,7 +45,7 @@ class DataUpdatesHooks implements RevisionDataUpdatesHook {
 			$method = __METHOD__;
 			$updates[] = new MWCallableUpdate( function () use ( $rev, $method ) {
 				try {
-					$threadItemSet = HookUtils::parseRevisionParsoidHtml( $rev, $method );
+					$threadItemSet = HookUtils::parseRevisionParsoidHtml( $rev, $method )->getValueOrThrow();
 					if ( !$this->threadItemStore->isDisabled() ) {
 						$this->threadItemStore->insertThreadItems( $rev, $threadItemSet );
 					}

includes/Hooks/EchoHooks.php

@@ -23,7 +23,6 @@ use MediaWiki\Extension\Notifications\Model\Event;
 use MediaWiki\Extension\Notifications\UserLocator;
 use MediaWiki\Registration\ExtensionRegistry;
 use MediaWiki\Revision\RevisionRecord;
-use Wikimedia\Parsoid\Core\ResourceLimitExceededException;
 
 class EchoHooks implements
 	BeforeCreateEchoEventHook,
@@ -143,9 +142,6 @@ class EchoHooks implements
 		}
 	}
 
-	/**
-	 * @throws ResourceLimitExceededException
-	 */
 	public function onEchoGetEventsForRevision( array &$events, RevisionRecord $revision, bool $isRevert ) {
 		if ( $isRevert ) {
 			return;

includes/Hooks/HookUtils.php

@@ -14,7 +14,7 @@ use MediaWiki\Context\IContextSource;
 use MediaWiki\Context\RequestContext;
 use MediaWiki\Extension\DiscussionTools\CommentParser;
 use MediaWiki\Extension\DiscussionTools\CommentUtils;
-use MediaWiki\Extension\DiscussionTools\ContentThreadItemSet;
+use MediaWiki\Extension\DiscussionTools\ContentThreadItemSetStatus;
 use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Output\OutputPage;
@@ -22,12 +22,12 @@ use MediaWiki\Page\ParserOutputAccess;
 use MediaWiki\Parser\ParserOptions;
 use MediaWiki\Registration\ExtensionRegistry;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Status\Status;
 use MediaWiki\Title\Title;
 use MediaWiki\Title\TitleValue;
 use MediaWiki\User\UserIdentity;
-use RuntimeException;
 use Wikimedia\Assert\Assert;
-use Wikimedia\Parsoid\Core\ResourceLimitExceededException;
+use Wikimedia\NormalizedException\NormalizedException;
 use Wikimedia\Parsoid\Utils\DOMCompat;
 use Wikimedia\Parsoid\Utils\DOMUtils;
 use Wikimedia\Rdbms\IDBAccessObject;
@@ -118,13 +118,12 @@ class HookUtils {
 	 *        Otherwise, it should be set to the name of the calling method (__METHOD__),
 	 *        so we can track what is causing parser cache writes.
 	 *
-	 * @return ContentThreadItemSet
-	 * @throws ResourceLimitExceededException
+	 * @return ContentThreadItemSetStatus
 	 */
 	public static function parseRevisionParsoidHtml(
 		RevisionRecord $revRecord,
 		$updateParserCacheFor
-	): ContentThreadItemSet {
+	): ContentThreadItemSetStatus {
 		$services = MediaWikiServices::getInstance();
 		$mainConfig = $services->getMainConfig();
 		$parserOutputAccess = $services->getParserOutputAccess();
@@ -152,14 +151,12 @@ class HookUtils {
 		);
 
 		if ( !$status->isOK() ) {
-			[ 'message' => $key, 'params' => $params ] = $status->getErrors()[0];
-			// XXX: HtmlOutputRendererHelper checks for a resource limit exceeded message as well,
-			// maybe we shouldn't be catching that so low down.  Re-throw for callers.
+			// This is currently the only expected failure, make the caller handle it
 			if ( $status->hasMessage( 'parsoid-resource-limit-exceeded' ) ) {
-				throw new ResourceLimitExceededException( $params[0] ?? '' );
+				return ContentThreadItemSetStatus::wrap( $status );
 			}
-			$message = wfMessage( $key, ...$params );
-			throw new RuntimeException( $message->inLanguage( 'en' )->useDatabase( false )->text() );
+			// Any other failures indicate a software bug, so throw an exception
+			throw new NormalizedException( ...Status::wrap( $status )->getPsr3MessageAndContext() );
 		}
 
 		$parserOutput = $status->getValue();
@@ -176,7 +173,7 @@ class HookUtils {
 		/** @var CommentParser $parser */
 		$parser = $services->getService( 'DiscussionTools.CommentParser' );
 		$title = TitleValue::newFromPage( $revRecord->getPage() );
-		return $parser->parse( $container, $title );
+		return ContentThreadItemSetStatus::newGood( $parser->parse( $container, $title ) );
 	}
 
 	/**

includes/Notifications/EventDispatcher.php

@@ -34,22 +34,15 @@ use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Title\Title;
 use MediaWiki\User\UserIdentity;
 use Wikimedia\Assert\Assert;
-use Wikimedia\Parsoid\Core\ResourceLimitExceededException;
 use Wikimedia\Parsoid\Utils\DOMCompat;
 use Wikimedia\Parsoid\Utils\DOMUtils;
 use Wikimedia\Rdbms\IDBAccessObject;
 
 class EventDispatcher {
-	/**
-	 * @throws ResourceLimitExceededException
-	 */
 	private static function getParsedRevision( RevisionRecord $revRecord ): ContentThreadItemSet {
-		return HookUtils::parseRevisionParsoidHtml( $revRecord, __METHOD__ );
+		return HookUtils::parseRevisionParsoidHtml( $revRecord, __METHOD__ )->getValueOrThrow();
 	}
 
-	/**
-	 * @throws ResourceLimitExceededException
-	 */
 	public static function generateEventsForRevision( array &$events, RevisionRecord $newRevRecord ): void {
 		$services = MediaWikiServices::getInstance();
 

maintenance/persistRevisionThreadItems.php

@@ -199,7 +199,7 @@ class PersistRevisionThreadItems extends Maintenance {
 				$rev->getPageAsLinkTarget()
 			);
 			if ( HookUtils::isAvailableForTitle( $title ) ) {
-				$threadItemSet = HookUtils::parseRevisionParsoidHtml( $rev, false );
+				$threadItemSet = HookUtils::parseRevisionParsoidHtml( $rev, false )->getValueOrThrow();
 
 				// Store permalink data (even when store is disabled - T334258)
 				$changed = $this->itemStore->insertThreadItems( $rev, $threadItemSet );