From be222d195da521a4a270ea6cc12f075197f91ca0 Mon Sep 17 00:00:00 2001 From: soyuka Date: Thu, 18 Dec 2025 09:21:10 +0100 Subject: [PATCH] fix(laravel): allow custom error handler for non-api operations fixes #7466 --- .../Eloquent/Metadata/ModelMetadata.php | 2 +- src/Laravel/Exception/ErrorHandler.php | 11 +- .../Tests/CustomExceptionHandlerTest.php | 161 ++++++++++++++++++ src/Laravel/composer.json | 2 +- src/Laravel/phpunit.xml.dist | 8 +- .../app/Exceptions/CustomHandler.php | 33 ++++ .../app/Exceptions/CustomHandlerException.php | 18 ++ .../app/Exceptions/CustomTestException.php | 18 ++ 8 files changed, 247 insertions(+), 6 deletions(-) create mode 100644 src/Laravel/Tests/CustomExceptionHandlerTest.php create mode 100644 src/Laravel/workbench/app/Exceptions/CustomHandler.php create mode 100644 src/Laravel/workbench/app/Exceptions/CustomHandlerException.php create mode 100644 src/Laravel/workbench/app/Exceptions/CustomTestException.php diff --git a/src/Laravel/Eloquent/Metadata/ModelMetadata.php b/src/Laravel/Eloquent/Metadata/ModelMetadata.php index 378754660b8..b8f4e17e2db 100644 --- a/src/Laravel/Eloquent/Metadata/ModelMetadata.php +++ b/src/Laravel/Eloquent/Metadata/ModelMetadata.php @@ -120,7 +120,7 @@ private function isColumnPrimaryKey(array $indexes, string $column): bool /** * Get the virtual (non-column) attributes for the given model. * - * @param array $columns + * @param list> $columns * * @return array */ diff --git a/src/Laravel/Exception/ErrorHandler.php b/src/Laravel/Exception/ErrorHandler.php index a0796b9843c..213ae49efb4 100644 --- a/src/Laravel/Exception/ErrorHandler.php +++ b/src/Laravel/Exception/ErrorHandler.php @@ -67,6 +67,16 @@ public function render($request, \Throwable $exception) $apiOperation = $this->initializeOperation($request); if (!$apiOperation) { + // For non-API operations, first check if any renderable callbacks on this + // ErrorHandler instance can handle the exception (issue #7466). + $response = $this->renderViaCallbacks($request, $exception); + + if ($response) { + return $response; + } + + // If no callbacks handled it, delegate to the decorated handler if available + // to preserve custom exception handler classes (issue #7058). return $this->decorated ? $this->decorated->render($request, $exception) : parent::render($request, $exception); } @@ -160,7 +170,6 @@ public function render($request, \Throwable $exception) try { $response = $this->apiPlatformController->__invoke($dup); - $this->decorated->render($dup, $exception); return $response; } catch (\Throwable $e) { diff --git a/src/Laravel/Tests/CustomExceptionHandlerTest.php b/src/Laravel/Tests/CustomExceptionHandlerTest.php new file mode 100644 index 00000000000..755ad65e68e --- /dev/null +++ b/src/Laravel/Tests/CustomExceptionHandlerTest.php @@ -0,0 +1,161 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Laravel\Tests; + +use ApiPlatform\Laravel\Test\ApiTestAssertionsTrait; +use Illuminate\Contracts\Config\Repository; +use Illuminate\Contracts\Debug\ExceptionHandler; +use Illuminate\Foundation\Application; +use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Request; +use Illuminate\Http\Response; +use Illuminate\Support\Facades\Route; +use Orchestra\Testbench\Concerns\WithWorkbench; +use Orchestra\Testbench\TestCase; +use Workbench\App\Exceptions\CustomHandler; +use Workbench\App\Exceptions\CustomHandlerException; +use Workbench\App\Exceptions\CustomTestException; +use Workbench\Database\Factories\BookFactory; + +/** + * Tests for issues #7058 and #7466. + * + * Ensures both custom exception handler classes and callbacks registered via renderable() + * work correctly for non-API routes while API Platform operations use their own error handling. + */ +class CustomExceptionHandlerTest extends TestCase +{ + use ApiTestAssertionsTrait; + use RefreshDatabase; + use WithWorkbench; + + protected static bool $customHandlerCalled = false; + protected static bool $useCustomHandlerClass = false; + + /** + * @param Application $app + */ + protected function defineEnvironment($app): void + { + tap($app['config'], function (Repository $config): void { + $config->set('app.debug', false); + $config->set('api-platform.resources', [app_path('Models'), app_path('ApiResource')]); + }); + } + + protected function resolveApplicationExceptionHandler($app): void + { + $handlerClass = self::$useCustomHandlerClass ? CustomHandler::class : \Illuminate\Foundation\Exceptions\Handler::class; + $app->singleton(ExceptionHandler::class, $handlerClass); + } + + protected function setUp(): void + { + parent::setUp(); + self::$customHandlerCalled = false; + + if (!self::$useCustomHandlerClass) { + $this->app->make(ExceptionHandler::class)->renderable(function (\Throwable $exception, Request $request) { + if ($exception instanceof CustomTestException) { + self::$customHandlerCalled = true; + + return new Response('Custom handler response', 418); + } + }); + } + + Route::get('/non-api-route', function () { + throw new CustomTestException('This should be handled by custom handler'); + }); + + Route::get('/non-api-route-regular', function () { + throw new \RuntimeException('Regular exception on non-API route'); + }); + + Route::get('/non-api-custom-handler', function () { + throw new CustomHandlerException('Should use custom handler class'); + }); + } + + public function testCustomExceptionHandlerIsCalledForNonApiRoutes(): void + { + $response = $this->get('/non-api-route'); + + $this->assertTrue(self::$customHandlerCalled, 'Custom exception handler should be called for non-API routes'); + $response->assertStatus(418); + $this->assertEquals('Custom handler response', $response->getContent()); + } + + public function testCustomExceptionHandlerIsNotCalledForApiRoutes(): void + { + BookFactory::new()->count(1)->create(); + + $response = $this->get('/api/books/non-existent-id', ['accept' => 'application/ld+json']); + + $this->assertFalse(self::$customHandlerCalled, 'Custom exception handler should NOT be called for API Platform operations'); + $response->assertStatus(404); + } + + public function testRegularExceptionOnNonApiRoute(): void + { + $response = $this->get('/non-api-route-regular'); + + $response->assertStatus(500); + } + + public function testApiPlatformExceptionHandlingStillWorks(): void + { + $response = $this->get('/api/books/invalid-id', ['accept' => 'application/ld+json']); + + $response->assertStatus(404); + $this->assertStringContainsString('application/', $response->headers->get('content-type')); + } + + public function testCustomHandlerClassWorksForNonApiRoutes(): void + { + self::$useCustomHandlerClass = true; + $this->refreshApplication(); + $this->setUpTraits(); + CustomHandler::$customRenderCalled = false; + + Route::get('/non-api-custom-handler-test', function () { + throw new CustomHandlerException('Should use custom handler class'); + }); + + $response = $this->get('/non-api-custom-handler-test'); + + $this->assertTrue(CustomHandler::$customRenderCalled, 'Custom handler class render() should be called'); + $response->assertStatus(419); + $this->assertEquals('Custom Handler Class Response', $response->getContent()); + + self::$useCustomHandlerClass = false; + } + + public function testCustomHandlerClassDoesNotInterceptApiRoutes(): void + { + self::$useCustomHandlerClass = true; + $this->refreshApplication(); + $this->setUpTraits(); + CustomHandler::$customRenderCalled = false; + + BookFactory::new()->count(1)->create(); + + $response = $this->get('/api/books/non-existent-id', ['accept' => 'application/ld+json']); + + $this->assertFalse(CustomHandler::$customRenderCalled, 'Custom handler class should not be called for API operations'); + $response->assertStatus(404); + + self::$useCustomHandlerClass = false; + } +} diff --git a/src/Laravel/composer.json b/src/Laravel/composer.json index d776f3b8617..5182488443b 100644 --- a/src/Laravel/composer.json +++ b/src/Laravel/composer.json @@ -58,7 +58,7 @@ "doctrine/dbal": "^4.0", "larastan/larastan": "^2.0 || ^3.0", "laravel/sanctum": "^4.0", - "orchestra/testbench": "^9.1", + "orchestra/testbench": "^10.1", "phpdocumentor/type-resolver": "^1.7", "phpstan/phpdoc-parser": "^1.29 || ^2.0", "phpunit/phpunit": "11.5.x-dev", diff --git a/src/Laravel/phpunit.xml.dist b/src/Laravel/phpunit.xml.dist index 13ed0e2c01d..3b174d1b23e 100644 --- a/src/Laravel/phpunit.xml.dist +++ b/src/Laravel/phpunit.xml.dist @@ -18,14 +18,16 @@ - - trigger_deprecation - + + trigger_deprecation + ./ ./Tests + ./workbench + ./public ./vendor diff --git a/src/Laravel/workbench/app/Exceptions/CustomHandler.php b/src/Laravel/workbench/app/Exceptions/CustomHandler.php new file mode 100644 index 00000000000..d9847b33754 --- /dev/null +++ b/src/Laravel/workbench/app/Exceptions/CustomHandler.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Workbench\App\Exceptions; + +use Illuminate\Foundation\Exceptions\Handler; +use Illuminate\Http\Response; + +class CustomHandler extends Handler +{ + public static bool $customRenderCalled = false; + + public function render($request, \Throwable $exception) + { + if ($exception instanceof CustomHandlerException) { + self::$customRenderCalled = true; + + return new Response('Custom Handler Class Response', 419); + } + + return parent::render($request, $exception); + } +} diff --git a/src/Laravel/workbench/app/Exceptions/CustomHandlerException.php b/src/Laravel/workbench/app/Exceptions/CustomHandlerException.php new file mode 100644 index 00000000000..75f9195e253 --- /dev/null +++ b/src/Laravel/workbench/app/Exceptions/CustomHandlerException.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Workbench\App\Exceptions; + +class CustomHandlerException extends \Exception +{ +} diff --git a/src/Laravel/workbench/app/Exceptions/CustomTestException.php b/src/Laravel/workbench/app/Exceptions/CustomTestException.php new file mode 100644 index 00000000000..1c850ed6215 --- /dev/null +++ b/src/Laravel/workbench/app/Exceptions/CustomTestException.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Workbench\App\Exceptions; + +class CustomTestException extends \Exception +{ +}