From a7d181f829f57c0e2a25468d97abd3dc7c147479 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Sat, 11 Apr 2026 13:27:05 +0000 Subject: [PATCH 1/3] feat: implement global error handling and reporting with detailed JSON responses --- src/App.php | 58 ++++++---- src/AppProxy.php | 30 ++++- src/Http/ErrorHandler.php | 55 +++++++++ src/Http/ExceptionHandler.php | 44 +++++++ .../ErrorHandling/GlobalErrorBootstrap.php | 109 ++++++++++++++++++ src/functions.php | 4 +- tests/Feature/AppErrorHandlingTest.php | 90 +++++++++++++++ .../Unit/Runtime/GlobalErrorBootstrapTest.php | 19 +++ 8 files changed, 384 insertions(+), 25 deletions(-) create mode 100644 src/Http/ErrorHandler.php create mode 100644 src/Http/ExceptionHandler.php create mode 100644 src/Runtime/ErrorHandling/GlobalErrorBootstrap.php create mode 100644 tests/Feature/AppErrorHandlingTest.php create mode 100644 tests/Unit/Runtime/GlobalErrorBootstrapTest.php diff --git a/src/App.php b/src/App.php index 2ec1a551..0541b5b5 100644 --- a/src/App.php +++ b/src/App.php @@ -5,12 +5,13 @@ namespace Phenix; use Amp\Cluster\Cluster; -use Amp\Http\Server\DefaultErrorHandler; use Amp\Http\Server\Driver\ConnectionLimitingClientFactory; use Amp\Http\Server\Driver\ConnectionLimitingServerSocketFactory; use Amp\Http\Server\Driver\SocketClientFactory; +use Amp\Http\Server\ExceptionHandler as ExceptionHandlerContract; use Amp\Http\Server\Middleware; use Amp\Http\Server\Middleware\CompressionMiddleware; +use Amp\Http\Server\Middleware\ExceptionHandlerMiddleware; use Amp\Http\Server\Middleware\ForwardedHeaderType; use Amp\Http\Server\RequestHandler; use Amp\Http\Server\Router; @@ -33,6 +34,8 @@ use Phenix\Facades\Config; use Phenix\Facades\Route; use Phenix\Http\Constants\Protocol; +use Phenix\Http\ErrorHandler as AppErrorHandler; +use Phenix\Http\ExceptionHandler as AppExceptionHandler; use Phenix\Logging\LoggerFactory; use Phenix\Runtime\Log; use Phenix\Scheduling\TimerRegistry; @@ -60,7 +63,9 @@ class App implements AppContract, Makeable protected bool $signalTrapping = true; - protected DefaultErrorHandler $errorHandler; + protected AppErrorHandler $errorHandler; + + protected ExceptionHandlerContract $exceptionHandler; protected Protocol $protocol = Protocol::HTTP; @@ -75,7 +80,8 @@ public function __construct(string $path) self::$path = $path; self::$container = new Container(); - $this->errorHandler = new DefaultErrorHandler(); + $this->errorHandler = new AppErrorHandler(); + $this->exceptionHandler = new AppExceptionHandler($this->errorHandler); } public function setup(): void @@ -242,18 +248,24 @@ protected function createServer(): SocketHttpServer } return SocketHttpServer::createForBehindProxy( - $this->logger, - ForwardedHeaderType::XForwardedFor, - $trustedProxies + logger: $this->logger, + headerType: ForwardedHeaderType::XForwardedFor, + trustedProxies: $trustedProxies, + exceptionHandler: $this->exceptionHandler ); } - return SocketHttpServer::createForDirectAccess($this->logger); + return SocketHttpServer::createForDirectAccess( + logger: $this->logger, + exceptionHandler: $this->exceptionHandler + ); } protected function createClusterServer(): SocketHttpServer { - $middleware = []; + $middleware = [ + new ExceptionHandlerMiddleware($this->exceptionHandler), + ]; $allowedMethods = Middleware\AllowedMethodsMiddleware::DEFAULT_ALLOWED_METHODS; if (extension_loaded('zlib')) { @@ -271,11 +283,11 @@ protected function createClusterServer(): SocketHttpServer $middleware[] = new Middleware\ForwardedMiddleware(ForwardedHeaderType::XForwardedFor, $trustedProxies); return new SocketHttpServer( - $this->logger, - Cluster::getServerSocketFactory(), - new SocketClientFactory($this->logger), - $middleware, - $allowedMethods, + logger: $this->logger, + serverSocketFactory: Cluster::getServerSocketFactory(), + clientFactory: new SocketClientFactory(logger: $this->logger), + middleware: $middleware, + allowedMethods: $allowedMethods, ); } @@ -283,22 +295,22 @@ protected function createClusterServer(): SocketHttpServer $connectionLimitPerIp = 10; $serverSocketFactory = new ConnectionLimitingServerSocketFactory( - new LocalSemaphore($connectionLimit), - Cluster::getServerSocketFactory(), + semaphore: new LocalSemaphore($connectionLimit), + socketServerFactory: Cluster::getServerSocketFactory(), ); $clientFactory = new ConnectionLimitingClientFactory( - new SocketClientFactory($this->logger), - $this->logger, - $connectionLimitPerIp, + clientFactory: new SocketClientFactory(logger: $this->logger), + logger: $this->logger, + connectionsPerIpLimit: $connectionLimitPerIp, ); return new SocketHttpServer( - $this->logger, - $serverSocketFactory, - $clientFactory, - $middleware, - $allowedMethods, + logger: $this->logger, + serverSocketFactory: $serverSocketFactory, + clientFactory: $clientFactory, + middleware: $middleware, + allowedMethods: $allowedMethods, ); } diff --git a/src/AppProxy.php b/src/AppProxy.php index 43992256..c2118d44 100644 --- a/src/AppProxy.php +++ b/src/AppProxy.php @@ -5,6 +5,9 @@ namespace Phenix; use Phenix\Contracts\App as AppContract; +use Phenix\Facades\Config; +use Phenix\Runtime\ErrorHandling\GlobalErrorBootstrap; +use Throwable; class AppProxy implements AppContract { @@ -18,16 +21,28 @@ public function __construct( public function run(): void { + $this->configureErrorReporting(); + + GlobalErrorBootstrap::register(); + if ($this->testingMode) { $this->app->disableSignalTrapping(); } - $this->app->run(); + try { + $this->app->run(); + } catch (Throwable $exception) { + GlobalErrorBootstrap::restore(); + + throw $exception; + } } public function stop(): void { $this->app->stop(); + + GlobalErrorBootstrap::restore(); } public function swap(string $key, object $concrete): void @@ -46,4 +61,17 @@ public function enableTestingMode(): self return $this; } + + private function configureErrorReporting(): void + { + $debug = Config::get('app.debug') === true; + + error_reporting(E_ALL); + ini_set('display_errors', $debug ? '1' : '0'); + ini_set('display_startup_errors', $debug ? '1' : '0'); + + if ($debug) { + ini_set('log_errors', '1'); + } + } } diff --git a/src/Http/ErrorHandler.php b/src/Http/ErrorHandler.php new file mode 100644 index 00000000..63b07eac --- /dev/null +++ b/src/Http/ErrorHandler.php @@ -0,0 +1,55 @@ + false, + 'error' => $message, + 'status' => $status, + ]; + + if ($this->shouldExposeDebugDetails()) { + $payload['debug'] = [ + 'reason' => $message, + 'path' => $request?->getUri()->getPath(), + ]; + } + + return $this->json($payload, $status, $reason); + } + + /** + * @param array $payload + */ + public function json(array $payload, int $status, ?string $reason = null): Response + { + $response = new Response( + headers: [ + 'content-type' => 'application/json', + ], + body: json_encode($payload, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + + $response->setStatus($status, $reason); + + return $response; + } + + public function shouldExposeDebugDetails(): bool + { + return Config::get('app.debug') === true; + } +} diff --git a/src/Http/ExceptionHandler.php b/src/Http/ExceptionHandler.php new file mode 100644 index 00000000..d8c2cc11 --- /dev/null +++ b/src/Http/ExceptionHandler.php @@ -0,0 +1,44 @@ + $request->getMethod(), + 'uri' => (string) $request->getUri(), + 'client' => $request->getClient()->getRemoteAddress()->toString(), + ]); + + if (! $this->errorHandler->shouldExposeDebugDetails()) { + return $this->errorHandler->handleError(status: HttpStatus::INTERNAL_SERVER_ERROR, request: $request); + } + + return $this->errorHandler->json(payload: [ + 'success' => false, + 'error' => $exception->getMessage(), + 'status' => HttpStatus::INTERNAL_SERVER_ERROR, + 'debug' => [ + 'exception' => $exception::class, + 'file' => $exception->getFile(), + 'line' => $exception->getLine(), + 'path' => $request->getUri()->getPath(), + ], + ], status: HttpStatus::INTERNAL_SERVER_ERROR); + } +} diff --git a/src/Runtime/ErrorHandling/GlobalErrorBootstrap.php b/src/Runtime/ErrorHandling/GlobalErrorBootstrap.php new file mode 100644 index 00000000..2cfdcdc4 --- /dev/null +++ b/src/Runtime/ErrorHandling/GlobalErrorBootstrap.php @@ -0,0 +1,109 @@ + $severity, + 'source' => 'php-error', + ]); + + throw $exception; + } + + public static function handleException(Throwable $exception): void + { + report($exception, [ + 'source' => 'uncaught-exception', + ]); + + if (self::$previousExceptionHandler !== null) { + (self::$previousExceptionHandler)($exception); + } + } + + public static function handleShutdown(): void + { + if (! self::$active) { + return; + } + + $error = error_get_last(); + + if ($error === null || ! in_array($error['type'], self::FATAL_ERRORS, true)) { + return; + } + + report(new ErrorException( + $error['message'], + 0, + $error['type'], + $error['file'], + $error['line'] + ), [ + 'severity' => $error['type'], + 'source' => 'fatal-error', + ]); + } +} diff --git a/src/functions.php b/src/functions.php index e1ff3d52..6f395a85 100644 --- a/src/functions.php +++ b/src/functions.php @@ -72,12 +72,14 @@ function value($value, ...$args) } if (! function_exists('report')) { - function report(Throwable $e): void + function report(Throwable $e, array $context = []): void { Log::error($e->getMessage(), [ + 'exception' => $e::class, 'file' => $e->getFile(), 'line' => $e->getLine(), 'trace' => $e->getTraceAsString(), + ...$context, ]); } } diff --git a/tests/Feature/AppErrorHandlingTest.php b/tests/Feature/AppErrorHandlingTest.php new file mode 100644 index 00000000..b78153f4 --- /dev/null +++ b/tests/Feature/AppErrorHandlingTest.php @@ -0,0 +1,90 @@ +app->run(); + + $response = $this->get('/fails'); + + $response + ->assertStatusCode(HttpStatus::INTERNAL_SERVER_ERROR) + ->assertJsonPath('success', false) + ->assertJsonPath('error', 'Internal Server Error') + ->assertJsonPath('status', HttpStatus::INTERNAL_SERVER_ERROR->value) + ->assertJsonMissingFragment(['error' => 'Sensitive failure']); + + expect($response->getDecodedBody())->not->toHaveKey('debug'); +}); + +it('returns debug JSON for server errors when debug is enabled', function (): void { + Config::set('app.debug', true); + + Route::get('/debug-fails', function (): Response { + throw new RuntimeError('Visible local failure'); + }); + + $this->app->run(); + + $this->get('/debug-fails') + ->assertStatusCode(HttpStatus::INTERNAL_SERVER_ERROR) + ->assertJsonPath('success', false) + ->assertJsonPath('error', 'Visible local failure') + ->assertJsonPath('status', HttpStatus::INTERNAL_SERVER_ERROR->value) + ->assertJsonPath('debug.exception', RuntimeError::class) + ->assertJsonPath('debug.path', '/debug-fails'); +}); + +it('returns debug JSON outside local when debug is enabled', function (): void { + Config::set('app.debug', true); + + Route::get('/production-fails', function (): Response { + throw new RuntimeError('Production secret'); + }); + + $this->app->run(); + + $response = $this->get('/production-fails'); + + $response + ->assertStatusCode(HttpStatus::INTERNAL_SERVER_ERROR) + ->assertJsonPath('success', false) + ->assertJsonPath('error', 'Production secret') + ->assertJsonPath('debug.exception', RuntimeError::class) + ->assertJsonPath('debug.path', '/production-fails'); +}); + +it('returns JSON for not found and method not allowed errors', function (): void { + Route::get('/json-only', fn (): Response => response()->json(['ok' => true])); + + $this->app->run(); + + $this->get('/missing') + ->assertNotFound() + ->assertJsonPath('success', false) + ->assertJsonPath('error', 'Not Found') + ->assertJsonPath('status', HttpStatus::NOT_FOUND->value); + + $this->post('/json-only') + ->assertStatusCode(HttpStatus::METHOD_NOT_ALLOWED) + ->assertJsonPath('success', false) + ->assertJsonPath('error', 'Method Not Allowed') + ->assertJsonPath('status', HttpStatus::METHOD_NOT_ALLOWED->value); +}); diff --git a/tests/Unit/Runtime/GlobalErrorBootstrapTest.php b/tests/Unit/Runtime/GlobalErrorBootstrapTest.php new file mode 100644 index 00000000..60da694c --- /dev/null +++ b/tests/Unit/Runtime/GlobalErrorBootstrapTest.php @@ -0,0 +1,19 @@ +once() + ->withArgs(function (string $message, array $context): bool { + return $message === 'Invalid runtime state' + && $context['exception'] === ErrorException::class + && $context['severity'] === E_WARNING + && $context['source'] === 'php-error'; + }); + + GlobalErrorBootstrap::handleError(E_WARNING, 'Invalid runtime state', __FILE__, __LINE__); +})->throws(ErrorException::class); From ba623609aa79a891fc744a7e11ce551fc77f63dd Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Sat, 11 Apr 2026 14:16:38 +0000 Subject: [PATCH 2/3] tests: add coverage --- src/AppProxy.php | 8 ++++---- ...orBootstrap.php => GlobalErrorHandler.php} | 12 +++++++++--- .../Unit/Runtime/GlobalErrorBootstrapTest.php | 19 ------------------- 3 files changed, 13 insertions(+), 26 deletions(-) rename src/Runtime/ErrorHandling/{GlobalErrorBootstrap.php => GlobalErrorHandler.php} (90%) delete mode 100644 tests/Unit/Runtime/GlobalErrorBootstrapTest.php diff --git a/src/AppProxy.php b/src/AppProxy.php index c2118d44..83279fed 100644 --- a/src/AppProxy.php +++ b/src/AppProxy.php @@ -6,7 +6,7 @@ use Phenix\Contracts\App as AppContract; use Phenix\Facades\Config; -use Phenix\Runtime\ErrorHandling\GlobalErrorBootstrap; +use Phenix\Runtime\ErrorHandling\GlobalErrorHandler; use Throwable; class AppProxy implements AppContract @@ -23,7 +23,7 @@ public function run(): void { $this->configureErrorReporting(); - GlobalErrorBootstrap::register(); + GlobalErrorHandler::register(); if ($this->testingMode) { $this->app->disableSignalTrapping(); @@ -32,7 +32,7 @@ public function run(): void try { $this->app->run(); } catch (Throwable $exception) { - GlobalErrorBootstrap::restore(); + GlobalErrorHandler::restore(); throw $exception; } @@ -42,7 +42,7 @@ public function stop(): void { $this->app->stop(); - GlobalErrorBootstrap::restore(); + GlobalErrorHandler::restore(); } public function swap(string $key, object $concrete): void diff --git a/src/Runtime/ErrorHandling/GlobalErrorBootstrap.php b/src/Runtime/ErrorHandling/GlobalErrorHandler.php similarity index 90% rename from src/Runtime/ErrorHandling/GlobalErrorBootstrap.php rename to src/Runtime/ErrorHandling/GlobalErrorHandler.php index 2cfdcdc4..012176bd 100644 --- a/src/Runtime/ErrorHandling/GlobalErrorBootstrap.php +++ b/src/Runtime/ErrorHandling/GlobalErrorHandler.php @@ -9,7 +9,7 @@ use function in_array; -class GlobalErrorBootstrap +class GlobalErrorHandler { private const FATAL_ERRORS = [ E_ERROR, @@ -84,13 +84,19 @@ public static function handleException(Throwable $exception): void } public static function handleShutdown(): void + { + self::handleShutdownError(error_get_last()); + } + + /** + * @param array{type: int, message: string, file: string, line: int}|null $error + */ + public static function handleShutdownError(array|null $error): void { if (! self::$active) { return; } - $error = error_get_last(); - if ($error === null || ! in_array($error['type'], self::FATAL_ERRORS, true)) { return; } diff --git a/tests/Unit/Runtime/GlobalErrorBootstrapTest.php b/tests/Unit/Runtime/GlobalErrorBootstrapTest.php deleted file mode 100644 index 60da694c..00000000 --- a/tests/Unit/Runtime/GlobalErrorBootstrapTest.php +++ /dev/null @@ -1,19 +0,0 @@ -once() - ->withArgs(function (string $message, array $context): bool { - return $message === 'Invalid runtime state' - && $context['exception'] === ErrorException::class - && $context['severity'] === E_WARNING - && $context['source'] === 'php-error'; - }); - - GlobalErrorBootstrap::handleError(E_WARNING, 'Invalid runtime state', __FILE__, __LINE__); -})->throws(ErrorException::class); From 62b5449ad40523e0347fd999be1e55d3ad4bd24f Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Sat, 11 Apr 2026 14:20:05 +0000 Subject: [PATCH 3/3] tests: add coverage --- tests/Unit/Runtime/GlobalErrorHandlerTest.php | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 tests/Unit/Runtime/GlobalErrorHandlerTest.php diff --git a/tests/Unit/Runtime/GlobalErrorHandlerTest.php b/tests/Unit/Runtime/GlobalErrorHandlerTest.php new file mode 100644 index 00000000..056b2097 --- /dev/null +++ b/tests/Unit/Runtime/GlobalErrorHandlerTest.php @@ -0,0 +1,111 @@ +once() + ->withArgs(function (string $message, array $context): bool { + return $message === 'Invalid runtime state' + && $context['exception'] === ErrorException::class + && $context['severity'] === E_WARNING + && $context['source'] === 'php-error'; + }); + + GlobalErrorHandler::handleError(E_WARNING, 'Invalid runtime state', __FILE__, __LINE__); +})->throws(ErrorException::class); + +it('reports uncaught exceptions', function (): void { + $exception = new RuntimeException('Unhandled runtime failure'); + + Log::shouldReceive('error') + ->once() + ->withArgs(function (string $message, array $context) use ($exception): bool { + return $message === 'Unhandled runtime failure' + && $context['exception'] === RuntimeException::class + && $context['file'] === $exception->getFile() + && $context['line'] === $exception->getLine() + && is_string($context['trace']) + && $context['source'] === 'uncaught-exception'; + }); + + GlobalErrorHandler::handleException($exception); +}); + +it('delegates uncaught exceptions to the previous exception handler', function (): void { + $delegated = false; + $exception = new RuntimeException('Delegated runtime failure'); + + set_exception_handler(function (Throwable $handled) use (&$delegated, $exception): void { + $delegated = $handled === $exception; + }); + + GlobalErrorHandler::register(); + + try { + Log::shouldReceive('error')->once(); + + GlobalErrorHandler::handleException($exception); + + expect($delegated)->toBeTrue(); + } finally { + GlobalErrorHandler::restore(); + restore_exception_handler(); + } +}); + +it('does not report shutdown errors when bootstrap is inactive', function (): void { + Log::shouldReceive('error')->never(); + + GlobalErrorHandler::handleShutdownError([ + 'type' => E_ERROR, + 'message' => 'Fatal failure', + 'file' => __FILE__, + 'line' => __LINE__, + ]); +}); + +it('does not report non fatal shutdown errors', function (): void { + GlobalErrorHandler::register(); + + Log::shouldReceive('error')->never(); + + GlobalErrorHandler::handleShutdownError([ + 'type' => E_WARNING, + 'message' => 'Non fatal warning', + 'file' => __FILE__, + 'line' => __LINE__, + ]); +}); + +it('reports fatal shutdown errors', function (int $severity): void { + GlobalErrorHandler::register(); + + Log::shouldReceive('error') + ->once() + ->withArgs(function (string $message, array $context) use ($severity): bool { + return $message === 'Fatal shutdown failure' + && $context['exception'] === ErrorException::class + && $context['severity'] === $severity + && $context['source'] === 'fatal-error'; + }); + + GlobalErrorHandler::handleShutdownError([ + 'type' => $severity, + 'message' => 'Fatal shutdown failure', + 'file' => __FILE__, + 'line' => __LINE__, + ]); +})->with([ + E_ERROR, + E_PARSE, + E_CORE_ERROR, + E_COMPILE_ERROR, +]);