Refine error behavior of bin/search index

Summary: Fixes T5991. If //all requested documents// failed to index, consider this a catastrophic failure and exit with an error code.

Test Plan:
  - Ran `bin/search index --type TASK`, observed successful exit despite a small number of un-indexable documents.
  - Ran `bin/search index PHID-TASK-xxx` for an invalid task, observed exception on exit after complete failure.
  - Ran normal indexing through daemons.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5991

Differential Revision: https://secure.phabricator.com/D14174

src/applications/search/index/PhabricatorSearchDocumentIndexer.php

@@ -42,55 +42,37 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject {
   }
 
   public function indexDocumentByPHID($phid, $context) {
-    try {
+    $this->setContext($context);
-      $this->setContext($context);
 
-      $document = $this->buildAbstractDocumentByPHID($phid);
+    $document = $this->buildAbstractDocumentByPHID($phid);
-      if ($document === null) {
+    if ($document === null) {
-        // This indexer doesn't build a document index, so we're done.
+      // This indexer doesn't build a document index, so we're done.
-        return $this;
+      return $this;
-      }
-
-      $object = $this->loadDocumentByPHID($phid);
-
-      // Automatically rebuild CustomField indexes if the object uses custom
-      // fields.
-      if ($object instanceof PhabricatorCustomFieldInterface) {
-        $this->indexCustomFields($document, $object);
-      }
-
-      // Automatically rebuild subscriber indexes if the object is subscribable.
-      if ($object instanceof PhabricatorSubscribableInterface) {
-        $this->indexSubscribers($document);
-      }
-
-      // Automatically build project relationships
-      if ($object instanceof PhabricatorProjectInterface) {
-        $this->indexProjects($document, $object);
-      }
-
-      $engine = PhabricatorSearchEngine::loadEngine();
-      try {
-        $engine->reindexAbstractDocument($document);
-      } catch (Exception $ex) {
-        phlog(
-          pht(
-            'Unable to index document %s with engine %s.',
-            $document->getPHID(),
-            get_class($engine)));
-        phlog($ex);
-      }
-
-      $this->dispatchDidUpdateIndexEvent($phid, $document);
-    } catch (Exception $ex) {
-      phlog(
-        pht(
-          'Unable to build document %s with indexer %s.',
-          $phid,
-          get_class($this)));
-      phlog($ex);
     }
 
+    $object = $this->loadDocumentByPHID($phid);
+
+    // Automatically rebuild CustomField indexes if the object uses custom
+    // fields.
+    if ($object instanceof PhabricatorCustomFieldInterface) {
+      $this->indexCustomFields($document, $object);
+    }
+
+    // Automatically rebuild subscriber indexes if the object is subscribable.
+    if ($object instanceof PhabricatorSubscribableInterface) {
+      $this->indexSubscribers($document);
+    }
+
+    // Automatically build project relationships
+    if ($object instanceof PhabricatorProjectInterface) {
+      $this->indexProjects($document, $object);
+    }
+
+    $engine = PhabricatorSearchEngine::loadEngine();
+    $engine->reindexAbstractDocument($document);
+
+    $this->dispatchDidUpdateIndexEvent($phid, $document);
+
     return $this;
   }
 

src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php

@@ -93,13 +93,26 @@ final class PhabricatorSearchManagementIndexWorkflow
     $bar = id(new PhutilConsoleProgressBar())
       ->setTotal(count($phids));
 
+    $any_success = false;
     $indexer = new PhabricatorSearchIndexer();
     foreach ($phids as $phid) {
-      $indexer->queueDocumentForIndexing($phid);
+      try {
+        $indexer->queueDocumentForIndexing($phid);
+        $any_success = true;
+      } catch (Exception $ex) {
+        phlog($ex);
+      }
+
       $bar->update(1);
     }
 
     $bar->done();
+
+    if (!$any_success) {
+      throw new Exception(
+        pht('Failed to rebuild search index for any documents.'));
+    }
+
   }
 
   private function loadPHIDsByNames(array $names) {

src/applications/search/worker/PhabricatorSearchWorker.php

@@ -8,8 +8,16 @@ final class PhabricatorSearchWorker extends PhabricatorWorker {
     $phid = idx($data, 'documentPHID');
     $context = idx($data, 'context');
 
-    id(new PhabricatorSearchIndexer())
+    try {
-      ->indexDocumentByPHID($phid, $context);
+      id(new PhabricatorSearchIndexer())
+        ->indexDocumentByPHID($phid, $context);
+    } catch (Exception $ex) {
+      throw new PhabricatorWorkerPermanentFailureException(
+        pht(
+          'Failed to update search index for document "%s": %s',
+          $phid,
+          $ex->getMessage()));
+    }
   }
 
 }