mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-21 14:22:41 +01:00
Improve top-level fatal exception handling in PHP 7+
Summary: Depends on D20137. Ref T13250. Ref T12101. In versions of PHP beyond 7, various engine errors are gradually changing from internal fatals or internal errors to `Throwables`, a superclass of `Exception`. This is generally a good change, but code written against PHP 5.x before `Throwable` was introduced may not catch these errors, even when the code is intended to be a top-level exception handler. (The double-catch pattern here and elsewhere is because `Throwable` does not exist in older PHP, so `catch (Throwable $ex)` catches nothing. The `Exception $ex` clause catches everything in old PHP, the `Throwable $ex` clause catches everything in newer PHP.) Generalize some `Exception` into `Throwable`. Test Plan: - Added a bogus function call to the rendering stack. - Before change: got a blank page. - After change: nice exception page. {F6205012} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13250, T12101 Differential Revision: https://secure.phabricator.com/D20138
This commit is contained in:
parent
e03079aaaa
commit
b9a1260ef5
3 changed files with 58 additions and 10 deletions
|
@ -286,6 +286,7 @@ final class AphrontApplicationConfiguration
|
|||
$original_exception = $ex;
|
||||
}
|
||||
|
||||
$response_exception = null;
|
||||
try {
|
||||
if ($original_exception) {
|
||||
$response = $this->handleThrowable($original_exception);
|
||||
|
@ -296,7 +297,13 @@ final class AphrontApplicationConfiguration
|
|||
$response->setRequest($request);
|
||||
|
||||
self::writeResponse($sink, $response);
|
||||
} catch (Exception $response_exception) {
|
||||
} catch (Exception $ex) {
|
||||
$response_exception = $ex;
|
||||
} catch (Throwable $ex) {
|
||||
$response_exception = $ex;
|
||||
}
|
||||
|
||||
if ($response_exception) {
|
||||
// If we encountered an exception while building a normal response, then
|
||||
// encountered another exception while building a response for the first
|
||||
// exception, just throw the original exception. It is more likely to be
|
||||
|
|
|
@ -315,7 +315,7 @@ final class PhabricatorStartup {
|
|||
*
|
||||
* @param string Brief description of the exception context, like
|
||||
* `"Rendering Exception"`.
|
||||
* @param Exception The exception itself.
|
||||
* @param Throwable The exception itself.
|
||||
* @param bool True if it's okay to show the exception's stack trace
|
||||
* to the user. The trace will always be logged.
|
||||
* @return exit This method **does not return**.
|
||||
|
@ -324,7 +324,7 @@ final class PhabricatorStartup {
|
|||
*/
|
||||
public static function didEncounterFatalException(
|
||||
$note,
|
||||
Exception $ex,
|
||||
$ex,
|
||||
$show_trace) {
|
||||
|
||||
$message = '['.$note.'/'.get_class($ex).'] '.$ex->getMessage();
|
||||
|
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
phabricator_startup();
|
||||
|
||||
$fatal_exception = null;
|
||||
try {
|
||||
PhabricatorStartup::beginStartupPhase('libraries');
|
||||
PhabricatorStartup::loadCoreLibraries();
|
||||
|
@ -12,25 +13,65 @@ try {
|
|||
PhabricatorStartup::beginStartupPhase('sink');
|
||||
$sink = new AphrontPHPHTTPSink();
|
||||
|
||||
// PHP introduced a "Throwable" interface in PHP 7 and began making more
|
||||
// runtime errors throw as "Throwable" errors. This is generally good, but
|
||||
// makes top-level exception handling that is compatible with both PHP 5
|
||||
// and PHP 7 a bit tricky.
|
||||
|
||||
// In PHP 5, "Throwable" does not exist, so "catch (Throwable $ex)" catches
|
||||
// nothing.
|
||||
|
||||
// In PHP 7, various runtime conditions raise an Error which is a Throwable
|
||||
// but NOT an Exception, so "catch (Exception $ex)" will not catch them.
|
||||
|
||||
// To cover both cases, we "catch (Exception $ex)" to catch everything in
|
||||
// PHP 5, and most things in PHP 7. Then, we "catch (Throwable $ex)" to catch
|
||||
// everything else in PHP 7. For the most part, we only need to do this at
|
||||
// the top level.
|
||||
|
||||
$main_exception = null;
|
||||
try {
|
||||
PhabricatorStartup::beginStartupPhase('run');
|
||||
AphrontApplicationConfiguration::runHTTPRequest($sink);
|
||||
} catch (Exception $ex) {
|
||||
$main_exception = $ex;
|
||||
} catch (Throwable $ex) {
|
||||
$main_exception = $ex;
|
||||
}
|
||||
|
||||
if ($main_exception) {
|
||||
$response_exception = null;
|
||||
try {
|
||||
$response = new AphrontUnhandledExceptionResponse();
|
||||
$response->setException($ex);
|
||||
$response->setException($main_exception);
|
||||
|
||||
PhabricatorStartup::endOutputCapture();
|
||||
$sink->writeResponse($response);
|
||||
} catch (Exception $response_exception) {
|
||||
// If we hit a rendering exception, ignore it and throw the original
|
||||
// exception. It is generally more interesting and more likely to be
|
||||
// the root cause.
|
||||
throw $ex;
|
||||
} catch (Exception $ex) {
|
||||
$response_exception = $ex;
|
||||
} catch (Throwable $ex) {
|
||||
$response_exception = $ex;
|
||||
}
|
||||
|
||||
// If we hit a rendering exception, ignore it and throw the original
|
||||
// exception. It is generally more interesting and more likely to be
|
||||
// the root cause.
|
||||
|
||||
if ($response_exception) {
|
||||
throw $main_exception;
|
||||
}
|
||||
}
|
||||
} catch (Exception $ex) {
|
||||
PhabricatorStartup::didEncounterFatalException('Core Exception', $ex, false);
|
||||
$fatal_exception = $ex;
|
||||
} catch (Throwable $ex) {
|
||||
$fatal_exception = $ex;
|
||||
}
|
||||
|
||||
if ($fatal_exception) {
|
||||
PhabricatorStartup::didEncounterFatalException(
|
||||
'Core Exception',
|
||||
$fatal_exception,
|
||||
false);
|
||||
}
|
||||
|
||||
function phabricator_startup() {
|
||||
|
|
Loading…
Reference in a new issue