[Laravel] Alternative to binding the error handler

[zjbarg]

Description
The laravel package service provider binds ApiPlatform\Laravel\Exception\ErrorHandler to Illuminate\Foundation\Exceptions\Handler.

When trying to setup api platform on an existing application, one does not expect the exception handler to be swapped.

It would be more powerful if (optionally) we can add the error handler behavior to our existing error handler instead of overriding it.

[soyuka]

I think that b0fa229 fixes the issue, do you have any other idea?

[zjbarg]

This is solving my problem currently.

[soyuka]

nice :)

[gisostallenberg]

I think the error handler should be decorated, not replaced.

I have a L11, which came out of L10. The upgrade documentation states "However, we do not recommend that Laravel 10 applications upgrading to Laravel 11 attempt to migrate their application structure, as Laravel 11 has been carefully tuned to also support the Laravel 10 application structure."

We register Sentry in the application error handler, which is the recommended way in the old application structure.
This is completely skipped when the API platform error handler replaces the apps error handler. So we do not have Sentry error handling in API platform.

Inspiration on how this is properly decorated can be seen at https://github.com/nunomaduro/collision/blob/60207965f9b7b7a4ce15a0f75d57f9dadb105bdb/src/Adapters/Laravel/ExceptionHandler.php, which is created in https://github.com/nunomaduro/collision/blob/60207965f9b7b7a4ce15a0f75d57f9dadb105bdb/src/Adapters/Laravel/CollisionServiceProvider.php#L61

The service provider creates the error handler and passes it to the decorator.

AFAIK this can also be done using Container::extend

$this->app->extend(
     \Illuminate\Contracts\Debug\ExceptionHandler::class,
    function (\Illuminate\Contracts\Debug\ExceptionHandler $decorated, \Illuminate\Contracts\Foundation\Application $app) {
        return new ApiPlatformsOwnHandler($decorated, /* ...... */);
    }
);

[soyuka]

@gisostallenberg indeed I like the idea and I thought about it but didn't thought that render would be use to collect data (eg sentry). Would you be able to try my changes at #7247 to see if they make a difference? I'm unconfortable calling render from the decoration without using its result :/. Shouldn't you collect sentry data inside report instead?

[gisostallenberg]

We changed the whole bootstrap/app.php to the new format in L11. This makes

    ->withExceptions(function (\Illuminate\Foundation\Configuration\Exceptions $exceptions) {
        \Sentry\Laravel\Integration::handles($exceptions);
    })

work properly with the current error handler in API platform.

It might be worth to add a warning somewhere, that this needs to be done, although I would not know where... Detect it in the API platform service provider? In the docs?