Diffusion - clean up catching ConduitException

Summary: Ref T7123. Turns out that we might throw ConduitClientException now in proxied scenarios. For all but one callsite remove the try / catch bit and don't issue the call for SVN. For the remaining callsite, also don't issue the call for SVN but keep in the exception logic since its renders a pretty error message in the non-proxied case?

Test Plan: played around with diffusion and things looked okay.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7123

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

src/applications/diffusion/controller/DiffusionBrowseSearchController.php

@@ -52,39 +52,37 @@ final class DiffusionBrowseSearchController extends DiffusionBrowseController {
 
     $search_mode = null;
 
-    try {
+    switch ($repository->getVersionControlSystem()) {
-      if (strlen($this->getRequest()->getStr('grep'))) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-        $search_mode = 'grep';
+        $results = array();
-        $query_string = $this->getRequest()->getStr('grep');
+        break;
-        $results = $this->callConduitWithDiffusionRequest(
+      default:
-          'diffusion.searchquery',
+        if (strlen($this->getRequest()->getStr('grep'))) {
-          array(
+          $search_mode = 'grep';
-            'grep' => $query_string,
+          $query_string = $this->getRequest()->getStr('grep');
-            'commit' => $drequest->getStableCommit(),
+          $results = $this->callConduitWithDiffusionRequest(
-            'path' => $drequest->getPath(),
+            'diffusion.searchquery',
-            'limit' => $limit + 1,
+            array(
-            'offset' => $page,
+              'grep' => $query_string,
-          ));
+              'commit' => $drequest->getStableCommit(),
-      } else { // Filename search.
+              'path' => $drequest->getPath(),
-        $search_mode = 'find';
+              'limit' => $limit + 1,
-        $query_string = $this->getRequest()->getStr('find');
+              'offset' => $page,
-        $results = $this->callConduitWithDiffusionRequest(
+            ));
-          'diffusion.querypaths',
+        } else { // Filename search.
-          array(
+          $search_mode = 'find';
-            'pattern' => $query_string,
+          $query_string = $this->getRequest()->getStr('find');
-            'commit' => $drequest->getStableCommit(),
+          $results = $this->callConduitWithDiffusionRequest(
-            'path' => $drequest->getPath(),
+            'diffusion.querypaths',
-            'limit' => $limit + 1,
+            array(
-            'offset' => $page,
+              'pattern' => $query_string,
-          ));
+              'commit' => $drequest->getStableCommit(),
-      }
+              'path' => $drequest->getPath(),
-    } catch (ConduitException $ex) {
+              'limit' => $limit + 1,
-      $err = $ex->getErrorDescription();
+              'offset' => $page,
-      if ($err != '') {
+            ));
-        return id(new PHUIErrorView())
+        }
-          ->setTitle(pht('Search Error'))
+        break;
-          ->appendChild($err);
-      }
     }
 
     $results = $pager->sliceResults($results);

src/applications/diffusion/controller/DiffusionCommitBranchesController.php

@@ -8,22 +8,22 @@ final class DiffusionCommitBranchesController extends DiffusionController {
 
   protected function processDiffusionRequest(AphrontRequest $request) {
     $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
 
-    $branches = array();
+    switch ($repository->getVersionControlSystem()) {
-    try {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-      $branches = $this->callConduitWithDiffusionRequest(
+        $branches = array();
-        'diffusion.branchquery',
+        break;
-        array(
+      default:
-          'contains' => $drequest->getCommit(),
+        $branches = $this->callConduitWithDiffusionRequest(
-        ));
+          'diffusion.branchquery',
-    } catch (ConduitException $ex) {
+          array(
-      if ($ex->getMessage() != 'ERR-UNSUPPORTED-VCS') {
+            'contains' => $drequest->getCommit(),
-        throw $ex;
+          ));
-      }
+        break;
     }
 
     $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches);
-
     $branch_links = array();
     foreach ($branches as $branch) {
       $branch_links[] = phutil_tag(

src/applications/diffusion/controller/DiffusionCommitTagsController.php

@@ -8,23 +8,23 @@ final class DiffusionCommitTagsController extends DiffusionController {
 
   protected function processDiffusionRequest(AphrontRequest $request) {
     $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
     $tag_limit = 10;
-
+    switch ($repository->getVersionControlSystem()) {
-    $tags = array();
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-    try {
+        $tags = array();
-      $tags = DiffusionRepositoryTag::newFromConduit(
+        break;
-        $this->callConduitWithDiffusionRequest(
+      default:
-          'diffusion.tagsquery',
+        $tags = DiffusionRepositoryTag::newFromConduit(
-          array(
+          $this->callConduitWithDiffusionRequest(
-            'commit' => $drequest->getCommit(),
+            'diffusion.tagsquery',
-            'limit' => $tag_limit + 1,
+            array(
-          )));
+              'commit' => $drequest->getCommit(),
-    } catch (ConduitException $ex) {
+              'limit' => $tag_limit + 1,
-      if ($ex->getMessage() != 'ERR-UNSUPPORTED-VCS') {
+            )));
-        throw $ex;
+        break;
-      }
     }
-
     $has_more_tags = (count($tags) > $tag_limit);
     $tags = array_slice($tags, 0, $tag_limit);
 

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -374,23 +374,23 @@ final class DiffusionRepositoryController extends DiffusionController {
 
   private function buildTagListTable(DiffusionRequest $drequest) {
     $viewer = $this->getRequest()->getUser();
+    $repository = $drequest->getRepository();
 
+    switch ($repository->getVersionControlSystem()) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        // no tags in SVN
+        return null;
+    }
     $tag_limit = 15;
     $tags = array();
-    try {
+    $tags = DiffusionRepositoryTag::newFromConduit(
-      $tags = DiffusionRepositoryTag::newFromConduit(
+      $this->callConduitWithDiffusionRequest(
-        $this->callConduitWithDiffusionRequest(
+        'diffusion.tagsquery',
-          'diffusion.tagsquery',
+        array(
-          array(
+          // On the home page, we want to find tags on any branch.
-            // On the home page, we want to find tags on any branch.
+          'commit' => null,
-            'commit' => null,
+          'limit' => $tag_limit + 1,
-            'limit' => $tag_limit + 1,
+        )));
-          )));
-    } catch (ConduitException $e) {
-      if ($e->getMessage() != 'ERR-UNSUPPORTED-VCS') {
-        throw $e;
-      }
-    }
 
     if (!$tags) {
       return null;
@@ -402,7 +402,7 @@ final class DiffusionRepositoryController extends DiffusionController {
     $commits = id(new DiffusionCommitQuery())
       ->setViewer($viewer)
       ->withIdentifiers(mpull($tags, 'getCommitIdentifier'))
-      ->withRepository($drequest->getRepository())
+      ->withRepository($repository)
       ->needCommitData(true)
       ->execute();
 

src/applications/diffusion/controller/DiffusionTagListController.php

@@ -28,16 +28,16 @@ final class DiffusionTagListController extends DiffusionController {
       $is_commit = false;
     }
 
-    $tags = array();
+    switch ($repository->getVersionControlSystem()) {
-    try {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-      $conduit_result = $this->callConduitWithDiffusionRequest(
+        $tags = array();
-        'diffusion.tagsquery',
+        break;
-        $params);
+      default:
-      $tags = DiffusionRepositoryTag::newFromConduit($conduit_result);
+        $conduit_result = $this->callConduitWithDiffusionRequest(
-    } catch (ConduitException $ex) {
+          'diffusion.tagsquery',
-      if ($ex->getMessage() != 'ERR-UNSUPPORTED-VCS') {
+          $params);
-        throw $ex;
+        $tags = DiffusionRepositoryTag::newFromConduit($conduit_result);
-      }
+        break;
     }
     $tags = $pager->sliceResults($tags);