In Webhooks, give errors human-readable labels and show reminder text for "Silent Mode"

Summary:
Depends on D19928. See <https://discourse.phabricator-community.org/t/firehose-webhook-not-working-with-self-hosted-requestbin-instance/2240/>.

Currently, we report "hook" and "silent", which are raw internal codes.

Instead, report human-readable labels so the user gets a better hint about what's going on ("In Silent Mode").

Also, render a "hey, you're in silent mode so none of this will work" reminder banner in this UI.

Test Plan:
{F6074421}

Note:

  - New warning banner.
  - Table has more human-readable text ("In Silent Mode").

Reviewers: amckinley

Reviewed By: amckinley

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

src/applications/herald/controller/HeraldWebhookViewController.php

@@ -50,6 +50,19 @@ final class HeraldWebhookViewController
       ->setLimit(20)
       ->execute();
 
+    $warnings = array();
+    if (PhabricatorEnv::getEnvConfig('phabricator.silent')) {
+      $message = pht(
+        'Phabricator is currently configured in silent mode, so it will not '.
+        'publish webhooks. To adjust this setting, see '.
+        '@{config:phabricator.silent} in Config.');
+
+      $warnings[] = id(new PHUIInfoView())
+        ->setTitle(pht('Silent Mode'))
+        ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
+        ->appendChild(new PHUIRemarkupView($viewer, $message));
+    }
+
     $requests_table = id(new HeraldWebhookRequestListView())
       ->setViewer($viewer)
       ->setRequests($requests)

src/applications/herald/storage/HeraldWebhookRequest.php

@@ -26,6 +26,15 @@ final class HeraldWebhookRequest
   const RESULT_OKAY = 'okay';
   const RESULT_FAIL = 'fail';
 
+  const ERRORTYPE_HOOK = 'hook';
+  const ERRORTYPE_HTTP = 'http';
+  const ERRORTYPE_TIMEOUT = 'timeout';
+
+  const ERROR_SILENT = 'silent';
+  const ERROR_DISABLED = 'disabled';
+  const ERROR_URI = 'uri';
+  const ERROR_OBJECT = 'object';
+
   protected function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
@@ -108,6 +117,28 @@ final class HeraldWebhookRequest
     return $this->getProperty('errorCode');
   }
 
+  public function getErrorTypeForDisplay() {
+    $map = array(
+      self::ERRORTYPE_HOOK => pht('Hook Error'),
+      self::ERRORTYPE_HTTP => pht('HTTP Error'),
+      self::ERRORTYPE_TIMEOUT => pht('Request Timeout'),
+    );
+
+    $type = $this->getErrorType();
+    return idx($map, $type, $type);
+  }
+
+  public function getErrorCodeForDisplay() {
+    $code = $this->getErrorCode();
+
+    if ($this->getErrorType() !== self::ERRORTYPE_HOOK) {
+      return $code;
+    }
+
+    $spec = $this->getHookErrorSpec($code);
+    return idx($spec, 'display', $code);
+  }
+
   public function setTransactionPHIDs(array $phids) {
     return $this->setProperty('transactionPHIDs', $phids);
   }
@@ -187,6 +218,28 @@ final class HeraldWebhookRequest
       ->setTooltip($tooltip);
   }
 
+  private function getHookErrorSpec($code) {
+    $map = $this->getHookErrorMap();
+    return idx($map, $code, array());
+  }
+
+  private function getHookErrorMap() {
+    return array(
+      self::ERROR_SILENT => array(
+        'display' => pht('In Silent Mode'),
+      ),
+      self::ERROR_DISABLED => array(
+        'display' => pht('Hook Disabled'),
+      ),
+      self::ERROR_URI => array(
+        'display' => pht('Invalid URI'),
+      ),
+      self::ERROR_OBJECT => array(
+        'display' => pht('Invalid Object'),
+      ),
+    );
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 

src/applications/herald/view/HeraldWebhookRequestListView.php

@@ -55,8 +55,8 @@ final class HeraldWebhookRequestListView
         $request->getID(),
         $icon,
         $handles[$request->getObjectPHID()]->renderLink(),
-        $request->getErrorType(),
+        $request->getErrorTypeForDisplay(),
-        $request->getErrorCode(),
+        $request->getErrorCodeForDisplay(),
         $last_request,
       );
     }
@@ -66,7 +66,7 @@ final class HeraldWebhookRequestListView
       ->setHeaders(
         array(
           pht('ID'),
-          '',
+          null,
           pht('Object'),
           pht('Type'),
           pht('Code'),

src/applications/herald/worker/HeraldWebhookWorker.php

@@ -35,14 +35,20 @@ final class HeraldWebhookWorker
     // If we're in silent mode, permanently fail the webhook request and then
     // return to complete this task.
     if (PhabricatorEnv::getEnvConfig('phabricator.silent')) {
-      $this->failRequest($request, 'hook', 'silent');
+      $this->failRequest(
+        $request,
+        HeraldWebhookRequest::ERRORTYPE_HOOK,
+        HeraldWebhookRequest::ERROR_SILENT);
       return;
     }
 
     $hook = $request->getWebhook();
 
     if ($hook->isDisabled()) {
-      $this->failRequest($request, 'hook', 'disabled');
+      $this->failRequest(
+        $request,
+        HeraldWebhookRequest::ERRORTYPE_HOOK,
+        HeraldWebhookRequest::ERROR_DISABLED);
       throw new PhabricatorWorkerPermanentFailureException(
         pht(
           'Associated hook ("%s") for webhook request ("%s") is disabled.',
@@ -59,7 +65,10 @@ final class HeraldWebhookWorker
           'https',
         ));
     } catch (Exception $ex) {
-      $this->failRequest($request, 'hook', 'uri');
+      $this->failRequest(
+        $request,
+        HeraldWebhookRequest::ERRORTYPE_HOOK,
+        HeraldWebhookRequest::ERROR_URI);
       throw new PhabricatorWorkerPermanentFailureException(
         pht(
           'Associated hook ("%s") for webhook request ("%s") has invalid '.
@@ -76,7 +85,11 @@ final class HeraldWebhookWorker
       ->withPHIDs(array($object_phid))
       ->executeOne();
     if (!$object) {
-      $this->failRequest($request, 'hook', 'object');
+      $this->failRequest(
+        $request,
+        HeraldWebhookRequest::ERRORTYPE_HOOK,
+        HeraldWebhookRequest::ERROR_OBJECT);
+
       throw new PhabricatorWorkerPermanentFailureException(
         pht(
           'Unable to load object ("%s") for webhook request ("%s").',
@@ -182,9 +195,9 @@ final class HeraldWebhookWorker
     list($status) = $future->resolve();
 
     if ($status->isTimeout()) {
-      $error_type = 'timeout';
+      $error_type = HeraldWebhookRequest::ERRORTYPE_TIMEOUT;
     } else {
-      $error_type = 'http';
+      $error_type = HeraldWebhookRequest::ERRORTYPE_HTTP;
     }
     $error_code = $status->getStatusCode();