Don't log Conduit 404s as errors
Summary: Fixes T5695. A Conduit "method does not exist" exception is somewhat expected... there is no need to `phlog` the exception. Test Plan: Called a non-existent Conduit method. Saw no exceptions in the error logs. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T5695 Differential Revision: https://secure.phabricator.com/D10042
This commit is contained in:
		| @@ -122,13 +122,15 @@ phutil_register_library_map(array( | |||||||
|     'ConduitAPIMethod' => 'applications/conduit/method/ConduitAPIMethod.php', |     'ConduitAPIMethod' => 'applications/conduit/method/ConduitAPIMethod.php', | ||||||
|     'ConduitAPIRequest' => 'applications/conduit/protocol/ConduitAPIRequest.php', |     'ConduitAPIRequest' => 'applications/conduit/protocol/ConduitAPIRequest.php', | ||||||
|     'ConduitAPIResponse' => 'applications/conduit/protocol/ConduitAPIResponse.php', |     'ConduitAPIResponse' => 'applications/conduit/protocol/ConduitAPIResponse.php', | ||||||
|  |     'ConduitApplicationNotInstalledException' => 'applications/conduit/protocol/exception/ConduitApplicationNotInstalledException.php', | ||||||
|     'ConduitCall' => 'applications/conduit/call/ConduitCall.php', |     'ConduitCall' => 'applications/conduit/call/ConduitCall.php', | ||||||
|     'ConduitCallTestCase' => 'applications/conduit/call/__tests__/ConduitCallTestCase.php', |     'ConduitCallTestCase' => 'applications/conduit/call/__tests__/ConduitCallTestCase.php', | ||||||
|     'ConduitConnectConduitAPIMethod' => 'applications/conduit/method/ConduitConnectConduitAPIMethod.php', |     'ConduitConnectConduitAPIMethod' => 'applications/conduit/method/ConduitConnectConduitAPIMethod.php', | ||||||
|     'ConduitConnectionGarbageCollector' => 'applications/conduit/garbagecollector/ConduitConnectionGarbageCollector.php', |     'ConduitConnectionGarbageCollector' => 'applications/conduit/garbagecollector/ConduitConnectionGarbageCollector.php', | ||||||
|     'ConduitException' => 'applications/conduit/protocol/ConduitException.php', |     'ConduitException' => 'applications/conduit/protocol/exception/ConduitException.php', | ||||||
|     'ConduitGetCertificateConduitAPIMethod' => 'applications/conduit/method/ConduitGetCertificateConduitAPIMethod.php', |     'ConduitGetCertificateConduitAPIMethod' => 'applications/conduit/method/ConduitGetCertificateConduitAPIMethod.php', | ||||||
|     'ConduitLogGarbageCollector' => 'applications/conduit/garbagecollector/ConduitLogGarbageCollector.php', |     'ConduitLogGarbageCollector' => 'applications/conduit/garbagecollector/ConduitLogGarbageCollector.php', | ||||||
|  |     'ConduitMethodNotFoundException' => 'applications/conduit/protocol/exception/ConduitMethodNotFoundException.php', | ||||||
|     'ConduitPingConduitAPIMethod' => 'applications/conduit/method/ConduitPingConduitAPIMethod.php', |     'ConduitPingConduitAPIMethod' => 'applications/conduit/method/ConduitPingConduitAPIMethod.php', | ||||||
|     'ConduitQueryConduitAPIMethod' => 'applications/conduit/method/ConduitQueryConduitAPIMethod.php', |     'ConduitQueryConduitAPIMethod' => 'applications/conduit/method/ConduitQueryConduitAPIMethod.php', | ||||||
|     'ConduitSSHWorkflow' => 'applications/conduit/ssh/ConduitSSHWorkflow.php', |     'ConduitSSHWorkflow' => 'applications/conduit/ssh/ConduitSSHWorkflow.php', | ||||||
| @@ -2854,12 +2856,14 @@ phutil_register_library_map(array( | |||||||
|       'Phobject', |       'Phobject', | ||||||
|       'PhabricatorPolicyInterface', |       'PhabricatorPolicyInterface', | ||||||
|     ), |     ), | ||||||
|  |     'ConduitApplicationNotInstalledException' => 'ConduitMethodNotFoundException', | ||||||
|     'ConduitCallTestCase' => 'PhabricatorTestCase', |     'ConduitCallTestCase' => 'PhabricatorTestCase', | ||||||
|     'ConduitConnectConduitAPIMethod' => 'ConduitAPIMethod', |     'ConduitConnectConduitAPIMethod' => 'ConduitAPIMethod', | ||||||
|     'ConduitConnectionGarbageCollector' => 'PhabricatorGarbageCollector', |     'ConduitConnectionGarbageCollector' => 'PhabricatorGarbageCollector', | ||||||
|     'ConduitException' => 'Exception', |     'ConduitException' => 'Exception', | ||||||
|     'ConduitGetCertificateConduitAPIMethod' => 'ConduitAPIMethod', |     'ConduitGetCertificateConduitAPIMethod' => 'ConduitAPIMethod', | ||||||
|     'ConduitLogGarbageCollector' => 'PhabricatorGarbageCollector', |     'ConduitLogGarbageCollector' => 'PhabricatorGarbageCollector', | ||||||
|  |     'ConduitMethodNotFoundException' => 'ConduitException', | ||||||
|     'ConduitPingConduitAPIMethod' => 'ConduitAPIMethod', |     'ConduitPingConduitAPIMethod' => 'ConduitAPIMethod', | ||||||
|     'ConduitQueryConduitAPIMethod' => 'ConduitAPIMethod', |     'ConduitQueryConduitAPIMethod' => 'ConduitAPIMethod', | ||||||
|     'ConduitSSHWorkflow' => 'PhabricatorSSHWorkflow', |     'ConduitSSHWorkflow' => 'PhabricatorSSHWorkflow', | ||||||
|   | |||||||
| @@ -172,16 +172,13 @@ final class ConduitCall { | |||||||
|     $method = ConduitAPIMethod::getConduitMethod($method_name); |     $method = ConduitAPIMethod::getConduitMethod($method_name); | ||||||
|  |  | ||||||
|     if (!$method) { |     if (!$method) { | ||||||
|       throw new ConduitException( |       throw new ConduitMethodNotFoundException($method); | ||||||
|         "Conduit method '{$method_name}' does not exist."); |  | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     $application = $method->getApplication(); |     $application = $method->getApplication(); | ||||||
|     if ($application && !$application->isInstalled()) { |     if ($application && !$application->isInstalled()) { | ||||||
|       $app_name = $application->getName(); |       $app_name = $application->getName(); | ||||||
|       throw new ConduitException( |       throw new ConduitApplicationNotInstalledException($method, $app_name); | ||||||
|         "Method '{$method_name}' belongs to application '{$app_name}', ". |  | ||||||
|         "which is not installed."); |  | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     return $method; |     return $method; | ||||||
|   | |||||||
| @@ -104,7 +104,9 @@ final class PhabricatorConduitAPIController | |||||||
|         list($error_code, $error_info) = $auth_error; |         list($error_code, $error_info) = $auth_error; | ||||||
|       } |       } | ||||||
|     } catch (Exception $ex) { |     } catch (Exception $ex) { | ||||||
|       phlog($ex); |       if (!($ex instanceof ConduitMethodNotFoundException)) { | ||||||
|  |         phlog($ex); | ||||||
|  |       } | ||||||
|       $result = null; |       $result = null; | ||||||
|       $error_code = ($ex instanceof ConduitException |       $error_code = ($ex instanceof ConduitException | ||||||
|         ? 'ERR-CONDUIT-CALL' |         ? 'ERR-CONDUIT-CALL' | ||||||
|   | |||||||
| @@ -0,0 +1,14 @@ | |||||||
|  | <?php | ||||||
|  |  | ||||||
|  | final class ConduitApplicationNotInstalledException | ||||||
|  |   extends ConduitMethodNotFoundException { | ||||||
|  |  | ||||||
|  |   public function __construct($method, $application) { | ||||||
|  |     parent::__construct( | ||||||
|  |       pht( | ||||||
|  |         "Method '%s' belongs to application '%s', which is not installed.", | ||||||
|  |         $method, | ||||||
|  |         $application)); | ||||||
|  |   } | ||||||
|  |  | ||||||
|  | } | ||||||
| @@ -1,6 +1,9 @@ | |||||||
| <?php | <?php | ||||||
| 
 | 
 | ||||||
| final class ConduitException extends Exception { | /** | ||||||
|  |  * @concrete-extensible | ||||||
|  |  */ | ||||||
|  | class ConduitException extends Exception { | ||||||
| 
 | 
 | ||||||
|   private $errorDescription; |   private $errorDescription; | ||||||
| 
 | 
 | ||||||
| @@ -12,7 +15,7 @@ final class ConduitException extends Exception { | |||||||
|    * @param string Detailed error description. |    * @param string Detailed error description. | ||||||
|    * @return this |    * @return this | ||||||
|    */ |    */ | ||||||
|   public function setErrorDescription($error_description) { |   final public function setErrorDescription($error_description) { | ||||||
|     $this->errorDescription = $error_description; |     $this->errorDescription = $error_description; | ||||||
|     return $this; |     return $this; | ||||||
|   } |   } | ||||||
| @@ -22,7 +25,7 @@ final class ConduitException extends Exception { | |||||||
|    * |    * | ||||||
|    * @return string|null Error description, if one is available. |    * @return string|null Error description, if one is available. | ||||||
|    */ |    */ | ||||||
|   public function getErrorDescription() { |   final public function getErrorDescription() { | ||||||
|     return $this->errorDescription; |     return $this->errorDescription; | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
| @@ -0,0 +1,12 @@ | |||||||
|  | <?php | ||||||
|  |  | ||||||
|  | /** | ||||||
|  |  * @concrete-extensible | ||||||
|  |  */ | ||||||
|  | class ConduitMethodNotFoundException extends ConduitException { | ||||||
|  |  | ||||||
|  |   public function __construct($method) { | ||||||
|  |     parent::__construct(pht("Conduit method '%s' does not exist.", $method)); | ||||||
|  |   } | ||||||
|  |  | ||||||
|  | } | ||||||
		Reference in New Issue
	
	Block a user
	 Joshua Spence
					Joshua Spence