From b8a5191e3b36f445f4bd5708472072a34427a0f1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 1 Jul 2020 06:12:09 -0700 Subject: [PATCH] Improve login/auth messages from Arcanist toolset workflows Summary: See PHI1802. After D21384, "arc land" and similar with no credentials now properly raise a useful exception, but it isn't formatted readably. Update the display code to make it look prettier. Test Plan: Ran "arc land" with no and invalid credentials, got properly formatted output. Differential Revision: https://secure.phabricator.com/D21387 --- src/__phutil_library_map__.php | 2 + src/conduit/ArcanistConduitCallFuture.php | 104 +++++++++++------- ...ArcanistConduitAuthenticationException.php | 27 +++++ src/runtime/ArcanistRuntime.php | 2 + 4 files changed, 94 insertions(+), 41 deletions(-) create mode 100644 src/exception/ArcanistConduitAuthenticationException.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9bd23305..90e2e2ef 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -126,6 +126,7 @@ phutil_register_library_map(array( 'ArcanistComprehensiveLintEngine' => 'lint/engine/ArcanistComprehensiveLintEngine.php', 'ArcanistConcatenationOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConcatenationOperatorXHPASTLinterRule.php', 'ArcanistConcatenationOperatorXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistConcatenationOperatorXHPASTLinterRuleTestCase.php', + 'ArcanistConduitAuthenticationException' => 'exception/ArcanistConduitAuthenticationException.php', 'ArcanistConduitCallFuture' => 'conduit/ArcanistConduitCallFuture.php', 'ArcanistConduitEngine' => 'conduit/ArcanistConduitEngine.php', 'ArcanistConduitException' => 'conduit/ArcanistConduitException.php', @@ -1170,6 +1171,7 @@ phutil_register_library_map(array( 'ArcanistComprehensiveLintEngine' => 'ArcanistLintEngine', 'ArcanistConcatenationOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistConcatenationOperatorXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistConduitAuthenticationException' => 'Exception', 'ArcanistConduitCallFuture' => 'FutureProxy', 'ArcanistConduitEngine' => 'Phobject', 'ArcanistConduitException' => 'Exception', diff --git a/src/conduit/ArcanistConduitCallFuture.php b/src/conduit/ArcanistConduitCallFuture.php index 01d9954d..187b4c27 100644 --- a/src/conduit/ArcanistConduitCallFuture.php +++ b/src/conduit/ArcanistConduitCallFuture.php @@ -15,61 +15,56 @@ final class ArcanistConduitCallFuture } private function raiseLoginRequired() { - $conduit_uri = $this->getEngine()->getConduitURI(); - $conduit_uri = new PhutilURI($conduit_uri); - $conduit_uri->setPath('/'); + $conduit_domain = $this->getConduitDomain(); - $conduit_domain = $conduit_uri->getDomain(); - - $block = id(new PhutilConsoleBlock()) - ->addParagraph( - tsprintf( - '** %s **', - pht('LOGIN REQUIRED'))) - ->addParagraph( + $message = array( + tsprintf( + "\n\n%W\n\n", pht( 'You are trying to connect to a server ("%s") that you do not '. 'have any stored credentials for, but the command you are '. 'running requires authentication.', - $conduit_domain)) - ->addParagraph( + $conduit_domain)), + tsprintf( + "%W\n\n", pht( - 'To login and save credentials for this server, run this '. - 'command:')) - ->addParagraph( - tsprintf( - " $ arc install-certificate %s\n", - $conduit_uri)); + 'To log in and save credentials for this server, run this '. + 'command:')), + tsprintf( + '%>', + $this->getInstallCommand()), + ); - throw new PhutilArgumentUsageException($block->drawConsoleString()); + $this->raiseException( + pht('Conduit API login required.'), + pht('LOGIN REQUIRED'), + $message); } private function raiseInvalidAuth() { - $conduit_uri = $this->getEngine()->getConduitURI(); - $conduit_uri = new PhutilURI($conduit_uri); - $conduit_uri->setPath('/'); + $conduit_domain = $this->getConduitDomain(); - $conduit_domain = $conduit_uri->getDomain(); - - $block = id(new PhutilConsoleBlock()) - ->addParagraph( - tsprintf( - '** %s **', - pht('INVALID CREDENTIALS'))) - ->addParagraph( + $message = array( + tsprintf( + "\n\n%W\n\n", pht( - 'Your stored credentials for this server ("%s") are not valid.', - $conduit_domain)) - ->addParagraph( + 'Your stored credentials for the server you are trying to connect '. + 'to ("%s") are not valid.', + $conduit_domain)), + tsprintf( + "%W\n\n", pht( - 'To login and save valid credentials for this server, run this '. - 'command:')) - ->addParagraph( - tsprintf( - " $ arc install-certificate %s\n", - $conduit_uri)); + 'To log in and save valid credentials for this server, run this '. + 'command:')), + tsprintf( + '%>', + $this->getInstallCommand()), + ); - throw new PhutilArgumentUsageException($block->drawConsoleString()); + $this->raiseException( + pht('Invalid Conduit API credentials.'), + pht('INVALID CREDENTIALS'), + $message); } protected function didReceiveResult($result) { @@ -91,4 +86,31 @@ final class ArcanistConduitCallFuture throw $exception; } + private function getInstallCommand() { + $conduit_uri = $this->getConduitURI(); + + return csprintf( + 'arc install-certificate %s', + $conduit_uri); + } + + private function getConduitURI() { + $conduit_uri = $this->getEngine()->getConduitURI(); + $conduit_uri = new PhutilURI($conduit_uri); + $conduit_uri->setPath('/'); + + return $conduit_uri; + } + + private function getConduitDomain() { + $conduit_uri = $this->getConduitURI(); + return $conduit_uri->getDomain(); + } + + private function raiseException($summary, $title, $body) { + throw id(new ArcanistConduitAuthenticationException($summary)) + ->setTitle($title) + ->setBody($body); + } + } diff --git a/src/exception/ArcanistConduitAuthenticationException.php b/src/exception/ArcanistConduitAuthenticationException.php new file mode 100644 index 00000000..72345453 --- /dev/null +++ b/src/exception/ArcanistConduitAuthenticationException.php @@ -0,0 +1,27 @@ +title = $title; + return $this; + } + + public function getTitle() { + return $this->title; + } + + public function setBody($body) { + $this->body = $body; + return $this; + } + + public function getBody() { + return $this->body; + } + +} diff --git a/src/runtime/ArcanistRuntime.php b/src/runtime/ArcanistRuntime.php index 485857f3..87d574a5 100644 --- a/src/runtime/ArcanistRuntime.php +++ b/src/runtime/ArcanistRuntime.php @@ -41,6 +41,8 @@ final class ArcanistRuntime { $log->writeError(pht('USAGE EXCEPTION'), $ex->getMessage()); } catch (ArcanistUserAbortException $ex) { $log->writeError(pht('---'), $ex->getMessage()); + } catch (ArcanistConduitAuthenticationException $ex) { + $log->writeError($ex->getTitle(), $ex->getBody()); } return 1;