Skip to content

Commit

Permalink
Handle fatal error as exception (#2136)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvorisek
mvorisek authored Dec 25, 2023
1 parent 492dab9 commit f27cc60
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 66 deletions.
2 changes: 1 addition & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@
},
"require-dev": {
"atk4/behat-mink-selenium2-driver": "^1.6.2",
"atk4/ergebnis-phpunit-slow-test-detector": "^2.4",
"behat/mink-extension": "^2.3.1",
"ergebnis/composer-normalize": "^2.13",
"ergebnis/phpunit-slow-test-detector": "^2.9",
"friendsofphp/php-cs-fixer": "^3.0",
"fzaninotto/faker": "^1.6",
"guzzlehttp/guzzle": "^7.3",
Expand Down
58 changes: 58 additions & 0 deletions demos/_unit-test/fatal-error.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

declare(strict_types=1);

namespace Atk4\Ui\Demos;

use Atk4\Ui\App;

/** @var App $app */
require_once __DIR__ . '/../init-app.php';

$runOnShutdownFx = static function (\Closure $fx) use ($app) {
// relies on https://github.com/atk4/ui/blob/5.0.0/src/App.php#L1108
$app->onHook(App::HOOK_BEFORE_RENDER, static function () use (&$fx) {
if ($fx !== null) {
try {
$fx();
} finally {
$fx = null;
}
}
});
};

$type = $app->tryGetRequestQueryParam('type');
if ($type === 'oom') {
ini_set('memory_limit', '16M');

$str = '';
for ($i = 0; $i < 1024; ++$i) {
$str .= random_bytes(16 * 1024);
}
} elseif ($type === 'time-limit') {
set_time_limit(1);

$str = '';
$t = microtime(true);
while (microtime(true) - $t < 1.5) {
$str = md5($str);
}
} elseif ($type === 'compile-error') {
// E_COMPILE_ERROR
// https://github.com/php/php-src/blob/php-8.3.0/Zend/zend_compile.c#L7459
// https://github.com/php/php-src/issues/9333
eval('class Cl { function foo(); }');
} elseif ($type === 'compile-warning') {
// E_COMPILE_WARNING
// https://github.com/php/php-src/blob/php-8.3.0/Zend/zend_compile.c#L6366
eval('declare(x=1);');
} elseif ($type === 'exception-in-shutdown') {
$runOnShutdownFx(static function () {
throw new \Exception('Exception from shutdown');
});
} elseif ($type === 'warning-in-shutdown') {
$runOnShutdownFx(static function () {
trigger_error('Warning from shutdown');
});
}
6 changes: 6 additions & 0 deletions phpstan.neon.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ parameters:
- '~^Only booleans are allowed in .+, .+ given( on the (left|right) side)?\.~'
- '~^Variable (static )?(property access|method call) on .+\.~'

# https://github.com/phpstan/phpstan/issues/10337
-
path: 'src/App.php'
message: '~^Call to method Atk4\\Ui\\App::callExit\(\) with true will always evaluate to true\.$~'
count: 2

# TODO these rules are generated, this ignores should be fixed in the code
# for level = 2
-
Expand Down
88 changes: 65 additions & 23 deletions src/App.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,15 @@ class App
use DynamicMethodTrait;
use HookTrait;

private const UNSUPPRESSEABLE_ERROR_LEVELS = \PHP_MAJOR_VERSION >= 8 ? (\E_ERROR | \E_PARSE | \E_CORE_ERROR | \E_COMPILE_ERROR | \E_USER_ERROR | \E_RECOVERABLE_ERROR) : 0;
private const UNHANDLEABLE_ERROR_LEVELS = \E_ERROR | \E_PARSE | \E_CORE_ERROR | \E_CORE_WARNING | \E_COMPILE_ERROR | \E_COMPILE_WARNING;
private const UNSUPPRESSIBLE_ERROR_LEVELS = \PHP_MAJOR_VERSION >= 8 ? (\E_ERROR | \E_PARSE | \E_CORE_ERROR | \E_COMPILE_ERROR | \E_USER_ERROR | \E_RECOVERABLE_ERROR) : 0;

public const HOOK_BEFORE_EXIT = self::class . '@beforeExit';
public const HOOK_BEFORE_RENDER = self::class . '@beforeRender';

private static ?string $shutdownReservedMemory; // @phpstan-ignore-line
private static ?int $errorReportingLevel = null;

/** @var array|false Location where to load JS/CSS files */
public $cdn = [
'atk' => '/public',
Expand Down Expand Up @@ -183,8 +187,13 @@ public function __construct(array $defaults = [])
// set our exception handler
if ($this->catchExceptions) {
set_exception_handler(\Closure::fromCallable([$this, 'caughtException']));
set_error_handler(static function (int $severity, string $msg, string $file, int $line): bool {
if ((error_reporting() & ~self::UNSUPPRESSEABLE_ERROR_LEVELS) === 0) {

$createErrorExceptionArgsFx = static function (int $severity, string $msg, string $file, int $line) {
return [$msg, 0, $severity, $file, $line];
};

set_error_handler(function (int $severity, string $msg, string $file, int $line) use ($createErrorExceptionArgsFx): bool {
if ((error_reporting() & ~self::UNSUPPRESSIBLE_ERROR_LEVELS) === 0) {
$isFirstFrame = true;
foreach (array_slice(debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS, 10), 1) as $frame) {
// allow to suppress any warning outside Atk4
Expand All @@ -203,8 +212,17 @@ public function __construct(array $defaults = [])
}
}

throw new \ErrorException($msg, 0, $severity, $file, $line);
$this->throwOrCaughtExceptionIfInShutdown(new \ErrorException(...$createErrorExceptionArgsFx($severity, $msg, $file, $line)));
});

register_shutdown_function(function () use ($createErrorExceptionArgsFx): void {
$error = error_get_last();
if ($error !== null && ($error['type'] & self::UNHANDLEABLE_ERROR_LEVELS) !== 0) {
$this->throwOrCaughtExceptionIfInShutdown(new \ErrorException(...$createErrorExceptionArgsFx($error['type'], $error['message'], $error['file'], $error['line'])));
}
});
self::$errorReportingLevel = error_reporting();
error_reporting(self::$errorReportingLevel & ~self::UNHANDLEABLE_ERROR_LEVELS);

http_response_code(500);
header('Content-Type: text/plain');
Expand Down Expand Up @@ -254,33 +272,46 @@ protected function setupTemplateDirs(): void
$this->templateDir[] = dirname(__DIR__) . '/template';
}

protected function callBeforeExit(): void
/**
* @return ($calledFromShutdownHandler is true ? void : never)
*/
public function callExit(bool $calledFromShutdownHandler = false): void
{
if (!$this->exitCalled) {
$this->exitCalled = true;
$this->hook(self::HOOK_BEFORE_EXIT);
}

if (!$calledFromShutdownHandler) {
if (!$this->callExit) {
// case process is not in shutdown mode
// App as already done everything
// App need to stop output
// set_handler to catch/trap any exception
set_exception_handler(static function (\Throwable $t): void {});

// raise exception to be trapped and stop execution
throw new ExitApplicationError();
}

exit;
}
}

/**
* @return never
*/
public function callExit(): void
private function throwOrCaughtExceptionIfInShutdown(\Throwable $exception): void
{
$this->callBeforeExit();

if (!$this->callExit) {
// case process is not in shutdown mode
// App as already done everything
// App need to stop output
// set_handler to catch/trap any exception
set_exception_handler(static function (\Throwable $t): void {});
if (\PHP_VERSION_ID < 80300 && self::$shutdownReservedMemory === null) {
// remove method once PHP 8.2 support is dropped
// https://github.com/php/php-src/issues/10695
$this->caughtException($exception);

// raise exception to be trapped and stop execution
throw new ExitApplicationError();
exit(1);
}

exit;
throw $exception;
}

protected function caughtException(\Throwable $exception): void
Expand Down Expand Up @@ -318,8 +349,7 @@ protected function caughtException(\Throwable $exception): void
}

// process is already in shutdown because of uncaught exception
// no need of call exit function
$this->callBeforeExit();
$this->callExit(true);
}

public function getRequest(): ServerRequestInterface
Expand Down Expand Up @@ -1098,7 +1128,7 @@ public function encodeJson($data, bool $forceObject = false): string
* $app->initLayout([Layout\Centered::class]);
* $app->layout->template->dangerouslySetHtml('Content', $e->getHtml());
* $app->run();
* $app->callBeforeExit();
* $app->callExit();
*/
public function renderExceptionHtml(\Throwable $exception): string
{
Expand All @@ -1111,15 +1141,14 @@ protected function setupAlwaysRun(): void
if (!$this->runCalled) {
try {
$this->run();
$this->callExit(true);
} catch (ExitApplicationError $e) {
// let the process continue and terminate using self::callExit() below
// continue shutdown
} catch (\Throwable $e) {
// set_exception_handler does not work in shutdown
// https://github.com/php/php-src/issues/10695
$this->caughtException($e);
}

$this->callBeforeExit();
}
});
}
Expand Down Expand Up @@ -1232,3 +1261,16 @@ private function outputResponseJson($data): void
$this->outputResponse($data);
}
}

\Closure::bind(static function (): void {
// reserve 1 MB of memory for shutdown
App::$shutdownReservedMemory = str_repeat(str_repeat('shutdownReserved', 256), 256);

register_shutdown_function(static function (): void {
App::$shutdownReservedMemory = null;

if (App::$errorReportingLevel !== null) {
error_reporting(error_reporting() | (App::$errorReportingLevel & App::UNHANDLEABLE_ERROR_LEVELS));
}
});
}, null, App::class)();
13 changes: 6 additions & 7 deletions tests/CallbackTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class CallbackTest extends TestCase
triggerCallback as private _triggerCallback;
}

/** @var string */
private $regexHtmlDoctype = '~^<!DOCTYPE html>\s*<html~';
protected static string $regexHtml = '~^<!DOCTYPE html>\s*<html.*</html>$~s';

protected function createApp(array $seed = []): App
{
Expand Down Expand Up @@ -159,7 +158,7 @@ public function testCallbackLater(): void

self::assertNull($var);

$this->expectOutputRegex($this->regexHtmlDoctype);
$this->expectOutputRegex(self::$regexHtml);
$cb->getApp()->run();
self::assertSame(34, $var);
}
Expand Down Expand Up @@ -195,7 +194,7 @@ public function testCallbackLaterNested(): void

self::assertNull($var);

$this->expectOutputRegex($this->regexHtmlDoctype);
$this->expectOutputRegex(self::$regexHtml);
$cb->getApp()->run();
self::assertSame(134, $var);
}
Expand All @@ -214,7 +213,7 @@ public function testCallbackLaterNotFiring(): void

self::assertNull($var);

$this->expectOutputRegex($this->regexHtmlDoctype);
$this->expectOutputRegex(self::$regexHtml);
$app->run();
self::assertNull($var); // @phpstan-ignore-line
}
Expand All @@ -235,7 +234,7 @@ public function testVirtualPage(): void
$var = 25;
});

$this->expectOutputRegex($this->regexHtmlDoctype);
$this->expectOutputRegex(self::$regexHtml);
$vp->getApp()->run();
self::assertSame(25, $var);
}
Expand All @@ -256,7 +255,7 @@ public function testVirtualPageCustomUrlTrigger(): void
$var = 25;
});

$this->expectOutputRegex($this->regexHtmlDoctype);
$this->expectOutputRegex(self::$regexHtml);
$vp->getApp()->run();
self::assertSame(25, $var);
}
Expand Down
31 changes: 31 additions & 0 deletions tests/DemosHttpTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,37 @@ protected function getPathWithAppVars(string $path): string
return parent::getPathWithAppVars($path);
}

#[\Override]
public static function provideDemoCallbackErrorCases(): iterable
{
yield from parent::provideDemoCallbackErrorCases();

yield [
'_unit-test/fatal-error.php?type=oom',
'Allowed memory size of 16777216 bytes exhausted',
];
yield [
'_unit-test/fatal-error.php?type=time-limit',
'Maximum execution time of 1 second exceeded',
];
yield [
'_unit-test/fatal-error.php?type=compile-error',
'Non-abstract method Cl::foo() must contain body',
];
yield [
'_unit-test/fatal-error.php?type=compile-warning',
'Unsupported declare \'x\'',
];
yield [
'_unit-test/fatal-error.php?type=exception-in-shutdown',
'Exception from shutdown',
];
yield [
'_unit-test/fatal-error.php?type=warning-in-shutdown',
'Warning from shutdown',
];
}

/**
* @dataProvider provideDemoLateOutputErrorCases
*/
Expand Down
Loading

0 comments on commit f27cc60

Please sign in to comment.