From da7d92dd0a4e11d305ad8634022a6226387d7e4e Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 26 Jan 2021 15:39:45 -0800 Subject: [PATCH] Catch more HTTP VCS errors and convert them into VCS repsonses Summary: Ref T13590. Currently, errors arising from cluster locking (like the "stuck write lock" exception) are not caught and converted into VCS responses on the HTTP VCS workflow. Catch a broader range of exceptions and convert them into appropriate responses. Test Plan: - Forced a "stuck write lock" exception, pushed to a Git repository over HTTP. - Before: generic fatal. - After: VCS-specific fatal with a useful message in the "X-Phabricator-Message" response header. Maniphest Tasks: T13590 Differential Revision: https://secure.phabricator.com/D21525 --- .../controller/DiffusionServeController.php | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index b9064863b1..d0d897e1f6 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -397,7 +397,25 @@ final class DiffusionServeController extends DiffusionController { switch ($vcs_type) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - $result = $this->serveVCSRequest($repository, $viewer); + $caught = null; + try { + $result = $this->serveVCSRequest($repository, $viewer); + } catch (Exception $ex) { + $caught = $ex; + } catch (Throwable $ex) { + $caught = $ex; + } + + if ($caught) { + // We never expect an uncaught exception here, so dump it to the + // log. All routine errors should have been converted into Response + // objects by a lower layer. + phlog($caught); + + $result = new PhabricatorVCSResponse( + 500, + phutil_string_cast($caught->getMessage())); + } break; case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: $result = new PhabricatorVCSResponse(