From 88798354e8c91554b15dd38b63732dfdda672a78 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 28 Mar 2017 15:10:59 -0700
Subject: [PATCH] Soften a possible cluster search setup fatal

Summary:
Ref T12450. The way that config repair and setup issues interact is kind of complicated, and if `cluster.search` is invalid we may end up using `cluster.search` before we repair it.

I poked at things for a bit but wasn't confident I could get it to consistently repair before we use it without doing a big messy change.

The only thing that really matters is whether "type" is valid or not, so just put a slightly softer/more-tailored check in for that.

Test Plan:
  - With `"type": "elastic"`, loaded setup issues.
  - Before patch: hard fatal.
  - After patch: softer fatal with more useful messaging.

{F4321048}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12450

Differential Revision: https://secure.phabricator.com/D17576
---
 .../PhabricatorClusterSearchConfigOptionType.php     |  2 +-
 .../cluster/search/PhabricatorSearchService.php      | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/infrastructure/cluster/config/PhabricatorClusterSearchConfigOptionType.php b/src/infrastructure/cluster/config/PhabricatorClusterSearchConfigOptionType.php
index 3c099a0548..90ead23e6d 100644
--- a/src/infrastructure/cluster/config/PhabricatorClusterSearchConfigOptionType.php
+++ b/src/infrastructure/cluster/config/PhabricatorClusterSearchConfigOptionType.php
@@ -4,7 +4,7 @@ final class PhabricatorClusterSearchConfigOptionType
   extends PhabricatorConfigJSONOptionType {
 
   public function validateOption(PhabricatorConfigOption $option, $value) {
-    self::validateClusterSearchConfigValue($value);
+    self::validateValue($value);
   }
 
   public static function validateValue($value) {
diff --git a/src/infrastructure/cluster/search/PhabricatorSearchService.php b/src/infrastructure/cluster/search/PhabricatorSearchService.php
index 2085f1d887..ed34f5cdf6 100644
--- a/src/infrastructure/cluster/search/PhabricatorSearchService.php
+++ b/src/infrastructure/cluster/search/PhabricatorSearchService.php
@@ -186,6 +186,18 @@ class PhabricatorSearchService
     $refs = array();
 
     foreach ($services as $config) {
+
+      // Normally, we've validated configuration before we get this far, but
+      // make sure we don't fatal if we end up here with a bogus configuration.
+      if (!isset($engines[$config['type']])) {
+        throw new Exception(
+          pht(
+            'Configured search engine type "%s" is unknown. Valid engines '.
+            'are: %s.',
+            $config['type'],
+            implode(', ', array_keys($engines))));
+      }
+
       $engine = $engines[$config['type']];
       $cluster = new self($engine);
       $cluster->setConfig($config);