1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-09 14:21:02 +01:00

Pass all Throwables to Exception Handlers, not just Exceptions

Summary:
Ref T12855. PHP7 introduced "Throwables", which are sort of like super exceptions. Some errors that PHP raises at runtime have become Throwables instead of old-school errors now.

The major effect this has is blank pages during development under PHP7 for certain classes of errors: they skip all the nice "show a pretty error" handlers and

This isn't a compelete fix, but catches the most common classes of unexpected Throwable and sends them through the normal machinery. Principally, it shows a nice stack trace again instead of a blank page for a larger class of typos and minor mistakes.

Test Plan:
Before: blank page. After:

{F5007979}

Reviewers: chad, amckinley

Reviewed By: chad

Maniphest Tasks: T12855

Differential Revision: https://secure.phabricator.com/D18136
This commit is contained in:
epriestley 2017-06-19 12:51:00 -07:00
parent 474d528c3b
commit c71d9c601f
9 changed files with 72 additions and 72 deletions

View file

@ -270,7 +270,10 @@ abstract class AphrontApplicationConfiguration extends Phobject {
} }
} catch (Exception $ex) { } catch (Exception $ex) {
$original_exception = $ex; $original_exception = $ex;
$response = $this->handleException($ex); $response = $this->handleThrowable($ex);
} catch (Throwable $ex) {
$original_exception = $ex;
$response = $this->handleThrowable($ex);
} }
try { try {
@ -663,24 +666,24 @@ abstract class AphrontApplicationConfiguration extends Phobject {
* This method delegates exception handling to available subclasses of * This method delegates exception handling to available subclasses of
* @{class:AphrontRequestExceptionHandler}. * @{class:AphrontRequestExceptionHandler}.
* *
* @param Exception Exception which needs to be handled. * @param Throwable Exception which needs to be handled.
* @return wild Response or response producer, or null if no available * @return wild Response or response producer, or null if no available
* handler can produce a response. * handler can produce a response.
* @task exception * @task exception
*/ */
private function handleException(Exception $ex) { private function handleThrowable($throwable) {
$handlers = AphrontRequestExceptionHandler::getAllHandlers(); $handlers = AphrontRequestExceptionHandler::getAllHandlers();
$request = $this->getRequest(); $request = $this->getRequest();
foreach ($handlers as $handler) { foreach ($handlers as $handler) {
if ($handler->canHandleRequestException($request, $ex)) { if ($handler->canHandleRequestThrowable($request, $throwable)) {
$response = $handler->handleRequestException($request, $ex); $response = $handler->handleRequestThrowable($request, $throwable);
$this->validateErrorHandlerResponse($handler, $response); $this->validateErrorHandlerResponse($handler, $response);
return $response; return $response;
} }
} }
throw $ex; throw $throwable;
} }
private static function newSelfCheckResponse() { private static function newSelfCheckResponse() {

View file

@ -12,19 +12,13 @@ abstract class AphrontRequestExceptionHandler extends Phobject {
abstract public function getRequestExceptionHandlerPriority(); abstract public function getRequestExceptionHandlerPriority();
public function shouldLogException( abstract public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable);
return null;
}
abstract public function canHandleRequestException( abstract public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex); $throwable);
abstract public function handleRequestException(
AphrontRequest $request,
Exception $ex);
final public static function getAllHandlers() { final public static function getAllHandlers() {
return id(new PhutilClassMapQuery()) return id(new PhutilClassMapQuery())

View file

@ -11,27 +11,28 @@ final class PhabricatorAjaxRequestExceptionHandler
return pht('Responds to requests made by AJAX clients.'); return pht('Responds to requests made by AJAX clients.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
// For non-workflow requests, return a Ajax response. // For non-workflow requests, return a Ajax response.
return ($request->isAjax() && !$request->isWorkflow()); return ($request->isAjax() && !$request->isWorkflow());
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
// Log these; they don't get shown on the client and can be difficult // Log these; they don't get shown on the client and can be difficult
// to debug. // to debug.
phlog($ex); phlog($throwable);
$response = new AphrontAjaxResponse(); $response = new AphrontAjaxResponse();
$response->setError( $response->setError(
array( array(
'code' => get_class($ex), 'code' => get_class($throwable),
'info' => $ex->getMessage(), 'info' => $throwable->getMessage(),
)); ));
return $response; return $response;
} }

View file

@ -11,19 +11,19 @@ final class PhabricatorConduitRequestExceptionHandler
return pht('Responds to requests made by Conduit clients.'); return pht('Responds to requests made by Conduit clients.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
return $request->isConduit(); return $request->isConduit();
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
$response = id(new ConduitAPIResponse()) $response = id(new ConduitAPIResponse())
->setErrorCode(get_class($ex)) ->setErrorCode(get_class($throwable))
->setErrorInfo($ex->getMessage()); ->setErrorInfo($throwable->getMessage());
return id(new AphrontJSONResponse()) return id(new AphrontJSONResponse())
->setAddJSONShield(false) ->setAddJSONShield(false)

View file

@ -11,9 +11,9 @@ final class PhabricatorDefaultRequestExceptionHandler
return pht('Handles all other exceptions.'); return pht('Handles all other exceptions.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
if (!$this->isPhabricatorSite($request)) { if (!$this->isPhabricatorSite($request)) {
return false; return false;
@ -22,9 +22,9 @@ final class PhabricatorDefaultRequestExceptionHandler
return true; return true;
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
$viewer = $this->getViewer($request); $viewer = $this->getViewer($request);
@ -33,18 +33,18 @@ final class PhabricatorDefaultRequestExceptionHandler
// the internet. These include requests with bad CSRF tokens and // the internet. These include requests with bad CSRF tokens and
// questionable "Host" headers. // questionable "Host" headers.
$should_log = true; $should_log = true;
if ($ex instanceof AphrontMalformedRequestException) { if ($throwable instanceof AphrontMalformedRequestException) {
$should_log = !$ex->getIsUnlogged(); $should_log = !$throwable->getIsUnlogged();
} }
if ($should_log) { if ($should_log) {
phlog($ex); phlog($throwable);
} }
$class = get_class($ex); $class = get_class($throwable);
$message = $ex->getMessage(); $message = $throwable->getMessage();
if ($ex instanceof AphrontSchemaQueryException) { if ($throwable instanceof AphrontSchemaQueryException) {
$message .= "\n\n".pht( $message .= "\n\n".pht(
"NOTE: This usually indicates that the MySQL schema has not been ". "NOTE: This usually indicates that the MySQL schema has not been ".
"properly upgraded. Run '%s' to ensure your schema is up to date.", "properly upgraded. Run '%s' to ensure your schema is up to date.",
@ -54,7 +54,7 @@ final class PhabricatorDefaultRequestExceptionHandler
if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
$trace = id(new AphrontStackTraceView()) $trace = id(new AphrontStackTraceView())
->setUser($viewer) ->setUser($viewer)
->setTrace($ex->getTrace()); ->setTrace($throwable->getTrace());
} else { } else {
$trace = null; $trace = null;
} }

View file

@ -13,26 +13,26 @@ final class PhabricatorHighSecurityRequestExceptionHandler
'to present MFA credentials to take an action.'); 'to present MFA credentials to take an action.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
if (!$this->isPhabricatorSite($request)) { if (!$this->isPhabricatorSite($request)) {
return false; return false;
} }
return ($ex instanceof PhabricatorAuthHighSecurityRequiredException); return ($throwable instanceof PhabricatorAuthHighSecurityRequiredException);
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
$viewer = $this->getViewer($request); $viewer = $this->getViewer($request);
$form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm( $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm(
$ex->getFactors(), $throwable->getFactors(),
$ex->getFactorValidationResults(), $throwable->getFactorValidationResults(),
$viewer, $viewer,
$request); $request);
@ -61,7 +61,7 @@ final class PhabricatorHighSecurityRequestExceptionHandler
'period of time. When you are finished taking sensitive '. 'period of time. When you are finished taking sensitive '.
'actions, you should leave high security.')) 'actions, you should leave high security.'))
->setSubmitURI($request->getPath()) ->setSubmitURI($request->getPath())
->addCancelButton($ex->getCancelURI()) ->addCancelButton($throwable->getCancelURI())
->addSubmitButton(pht('Enter High Security')); ->addSubmitButton(pht('Enter High Security'));
$request_parameters = $request->getPassthroughRequestParameters( $request_parameters = $request->getPassthroughRequestParameters(

View file

@ -13,20 +13,20 @@ final class PhabricatorPolicyRequestExceptionHandler
'do something they do not have permission to do.'); 'do something they do not have permission to do.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
if (!$this->isPhabricatorSite($request)) { if (!$this->isPhabricatorSite($request)) {
return false; return false;
} }
return ($ex instanceof PhabricatorPolicyException); return ($throwable instanceof PhabricatorPolicyException);
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
$viewer = $this->getViewer($request); $viewer = $this->getViewer($request);
@ -52,12 +52,12 @@ final class PhabricatorPolicyRequestExceptionHandler
array( array(
'class' => 'aphront-policy-rejection', 'class' => 'aphront-policy-rejection',
), ),
$ex->getRejection()), $throwable->getRejection()),
); );
$list = null; $list = null;
if ($ex->getCapabilityName()) { if ($throwable->getCapabilityName()) {
$list = $ex->getMoreInfo(); $list = $throwable->getMoreInfo();
foreach ($list as $key => $item) { foreach ($list as $key => $item) {
$list[$key] = $item; $list[$key] = $item;
} }
@ -67,12 +67,14 @@ final class PhabricatorPolicyRequestExceptionHandler
array( array(
'class' => 'aphront-capability-details', 'class' => 'aphront-capability-details',
), ),
pht('Users with the "%s" capability:', $ex->getCapabilityName())); pht(
'Users with the "%s" capability:',
$throwable->getCapabilityName()));
} }
$dialog = id(new AphrontDialogView()) $dialog = id(new AphrontDialogView())
->setTitle($ex->getTitle()) ->setTitle($throwable->getTitle())
->setClass('aphront-access-dialog') ->setClass('aphront-access-dialog')
->setUser($viewer) ->setUser($viewer)
->appendChild($content); ->appendChild($content);

View file

@ -13,20 +13,20 @@ final class PhabricatorRateLimitRequestExceptionHandler
'does something too frequently.'); 'does something too frequently.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
if (!$this->isPhabricatorSite($request)) { if (!$this->isPhabricatorSite($request)) {
return false; return false;
} }
return ($ex instanceof PhabricatorSystemActionRateLimitException); return ($throwable instanceof PhabricatorSystemActionRateLimitException);
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
$viewer = $this->getViewer($request); $viewer = $this->getViewer($request);
@ -34,8 +34,8 @@ final class PhabricatorRateLimitRequestExceptionHandler
->setTitle(pht('Slow Down!')) ->setTitle(pht('Slow Down!'))
->setUser($viewer) ->setUser($viewer)
->setErrors(array(pht('You are being rate limited.'))) ->setErrors(array(pht('You are being rate limited.')))
->appendParagraph($ex->getMessage()) ->appendParagraph($throwable->getMessage())
->appendParagraph($ex->getRateExplanation()) ->appendParagraph($throwable->getRateExplanation())
->addCancelButton('/', pht('Okaaaaaaaaaaaaaay...')); ->addCancelButton('/', pht('Okaaaaaaaaaaaaaay...'));
} }

View file

@ -11,24 +11,24 @@ final class PhabricatorClusterExceptionHandler
return pht('Handles runtime problems with cluster configuration.'); return pht('Handles runtime problems with cluster configuration.');
} }
public function canHandleRequestException( public function canHandleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
return ($ex instanceof PhabricatorClusterException); return ($throwable instanceof PhabricatorClusterException);
} }
public function handleRequestException( public function handleRequestThrowable(
AphrontRequest $request, AphrontRequest $request,
Exception $ex) { $throwable) {
$viewer = $this->getViewer($request); $viewer = $this->getViewer($request);
$title = $ex->getExceptionTitle(); $title = $throwable->getExceptionTitle();
$dialog = id(new AphrontDialogView()) $dialog = id(new AphrontDialogView())
->setTitle($title) ->setTitle($title)
->setUser($viewer) ->setUser($viewer)
->appendParagraph($ex->getMessage()) ->appendParagraph($throwable->getMessage())
->addCancelButton('/', pht('Proceed With Caution')); ->addCancelButton('/', pht('Proceed With Caution'));
return id(new AphrontDialogResponse()) return id(new AphrontDialogResponse())