feat(laravel): split render logic from error handler

[bonroyage]

Q	A
Branch?	main
Tickets	n/a
License	MIT
Doc PR

I have run into issues with the error handler a few times already. This PR aims to give the developer more control over error handling.

For the sake of clarity, let's define 2 terms:

• extended handler: API platform's handler that is instantiated in ApiPlatformDeferredProvider
• original handler: the handler that existed before extending and is passed to the extended handler

Factual observations:

• The Illuminate\Contracts\Debug\ExceptionHandler gets extended in the service provider, which passes the original handler to the extended handler as $decorated.
• The extended handler checks if it is dealing with an API operation.
  • If it is, it runs the API platform logic.
  • If it is not, it checks if there is a decorated handler
    • If there is, it calls the decorated handler's render method
    • If there is not, it calls the render method of the parent of the extended handler

Let's assume we add our own logic to withExceptions in bootstrap/app.php, everything defined in there is applied to the extended handler and not the original handler. So when falling through because it's not an API operation, and due to the fact that the decorated handler exists, map and respond doesn't work.

With the changes in this PR, the ErrorHandler is split up from the ErrorRenderer and a config is created to disable extending the original handler. My reasoning behind this is to allow developers to maintain full control over the exception handler. The ErrorRenderer can then still be called by the developer to run that logic when dealing with API operations.

The config has defaults that doesn't change anything for users. You'd have to explicitly disable the current behaviour.

In case you disable it, you can call the ErrorRenderer yourself in respond. For example:

$exceptions->respond(function (Response $response, Throwable $exception, Request $request) {
    if ($this->apiPlatformRenderer->shouldRender($request, $exception)) {
        return $this->apiPlatformRenderer->render($request, $exception)
            ?? $response;
    }

    // Other custom logic
})

Reproduction

On a clean Laravel installation, create two exceptions (MyException and MyOtherException).

// bootstrap/app.php
withExceptions(function (Exceptions $exceptions): void {
    $exceptions->map(MyException::class, MyOtherException::class);

    // or

    $exceptions->respond(function(Response $response, Throwable $e, Request $request) {
        return response()->view('error', [
            'status' => $response->getStatusCode(),
            'title' => get_class($e),
            'message' => $e->getMessage(),
        ]);
    });
})

Install api-platform/laravel, in both scenarios:

[soyuka]

Interesting I like the idea, but I'm wondering if there's another possibility to keep things properly separated with the actual exception system other than adding a flag :/.

Wouldn't the behavior you're describing be fixed if we delegated to the extended handler rather then to the decorated one?

Thanks for looking into this!

[bonroyage]

I think the decorated exists for a reason though. It's theoretically possible that the handler was extended once before or an entirely custom handler is created by the developer. If we were to simply delegate back to the extended handler, then we'd lose everything that's in those original handlers.

I considered proxying all calls to the original/decorated handler, but I'm not sure if that's the way to go. See below for a slimmed down snippet.

<?php

namespace ApiPlatform\Laravel\Exception;

/**
 * @mixin \Illuminate\Foundation\Exceptions\Handler
 */
class ErrorHandler implements ExceptionHandler
{
    use ForwardsCalls;
    use ContentNegotiationTrait;
    use OperationRequestInitiatorTrait;

    public function __construct(
        ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory,
        private readonly ApiPlatformController $apiPlatformController,
        private readonly ?IdentifiersExtractorInterface $identifiersExtractor = null,
        private readonly ?ResourceClassResolverInterface $resourceClassResolver = null,
        ?Negotiator $negotiator = null,
        private readonly ?array $exceptionToStatus = null,
        private readonly ?bool $debug = false,
        private readonly ?array $errorFormats = null,
        private readonly ExceptionHandler $decorated, // <--- this shouldn't be nullable anymore
    ) {
        $this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory;
        $this->negotiator = $negotiator;
    }

    public function render($request, \Throwable $exception)
    {
        $apiOperation = $this->initializeOperation($request);

        if (! $apiOperation) {
            return $this->decorated->render($request, $exception);
        }

        // ...

        try {
            $response = $this->apiPlatformController->__invoke($dup);

            return $response;
        } catch (\Throwable $e) {
            return $this->decorated->render($request, $exception);
        }
    }

    // ...

    public function report(Throwable $e)
    {
        return $this->decorated->report($e);
    }

    public function shouldReport(Throwable $e)
    {
        return $this->decorated->shouldReport($e);
    }

    public function renderForConsole($output, Throwable $e)
    {
        return $this->decorated->renderForConsole($output, $e);
    }

    public function __call(string $name, array $arguments)
    {
        return $this->forwardDecoratedCallTo($this->decorated, $name, $arguments);
    }
}

[soyuka]

I think that I tried that solution already but when laravel 12 got released it broke. I think that we'll go with your solution.