search

statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
3.71k stars 508 forks source link

Exception doesn't go through Handler #4486

Closed helloiamlukas closed 8 months ago

helloiamlukas commented 2 years ago

Bug description

I came across some exception that will be ignored by the app\Exceptions\Handler.php and will be directly handled by Illuminate\Foundation\Exceptions\Handler.

Therefore logging to external services (e.g. Sentry) will not work.

I don't know if this is Statamic specific or is a problem related to Laravel, but I thought I'm gonna open an issue so you can have a look.

How to reproduce

  1. Set up a fresh statamic/starter-kit-starters-creek statamic instance.
  2. Enable the Collection API and disable the API Cache in config/statamic/api.php
  3. Open /api/collections/blog/entries?filter[date:is_past]=abc
  4. Error will be shown.
  5. Add the following code to your app\Exceptions\Handler.php: 
    public function report(Throwable $e)
{
    dd('An error happend!');
}
  6. Open /api/collections/blog/entries?filter[date:is_past]=abc again
  7. Error will still be shown, our dd() will be ignored.

Logs

[2021-10-14 12:42:05] local.ERROR: Could not parse 'abc': DateTime::__construct(): Failed to parse time string (abc) at position 0 (a): The timezone could not be found in the database {"exception":"[object] (Carbon\\Exceptions\\InvalidFormatException(code: 0): Could not parse 'abc': DateTime::__construct(): Failed to parse time string (abc) at position 0 (a): The timezone could not be found in the database at /mysite/vendor/nesbot/carbon/src/Carbon/Traits/Creator.php:189)
[stacktrace]
#0 /mysite/vendor/nesbot/carbon/src/Carbon/Traits/Creator.php(215): Carbon\\Carbon::rawParse('abc', NULL)
#1 /mysite/vendor/statamic/cms/src/Tags/Concerns/QueriesConditions.php(277): Carbon\\Carbon::parse('abc')
#2 /mysite/vendor/statamic/cms/src/Tags/Concerns/QueriesConditions.php(120): Statamic\\Http\\Controllers\\API\\ApiController->queryIsBeforeCondition(Object(Statamic\\Stache\\Query\\EntryQueryBuilder), 'date', 'abc')
#3 /mysite/vendor/statamic/cms/src/Http/Controllers/API/ApiController.php(133): Statamic\\Http\\Controllers\\API\\ApiController->queryCondition(Object(Statamic\\Stache\\Query\\EntryQueryBuilder), 'date', 'is_past', 'abc')
#4 /mysite/vendor/laravel/framework/src/Illuminate/Collections/Traits/EnumeratesValues.php(234): Statamic\\Http\\Controllers\\API\\ApiController->Statamic\\Http\\Controllers\\API\\{closure}('abc', 'date:is_past')
#5 /mysite/vendor/statamic/cms/src/Http/Controllers/API/ApiController.php(134): Illuminate\\Support\\Collection->each(Object(Closure))
#6 /mysite/vendor/statamic/cms/src/Http/Controllers/API/ApiController.php(103): Statamic\\Http\\Controllers\\API\\ApiController->filter(Object(Statamic\\Stache\\Query\\EntryQueryBuilder))
#7 /mysite/vendor/statamic/cms/src/Http/Controllers/API/CollectionEntriesController.php(18): Statamic\\Http\\Controllers\\API\\ApiController->filterSortAndPaginate(Object(Statamic\\Stache\\Query\\EntryQueryBuilder))
#8 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): Statamic\\Http\\Controllers\\API\\CollectionEntriesController->index(Object(Statamic\\Entries\\Collection))
#9 /mysite/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(45): Illuminate\\Routing\\Controller->callAction('index', Array)
#10 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Route.php(262): Illuminate\\Routing\\ControllerDispatcher->dispatch(Object(Illuminate\\Routing\\Route), Object(Statamic\\Http\\Controllers\\API\\CollectionEntriesController), 'index')
#11 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Route.php(205): Illuminate\\Routing\\Route->runController()
#12 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Router.php(695): Illuminate\\Routing\\Route->run()
#13 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(128): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#14 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Middleware/SubstituteBindings.php(50): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#15 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Routing\\Middleware\\SubstituteBindings->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#16 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php(127): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#17 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php(103): Illuminate\\Routing\\Middleware\\ThrottleRequests->handleRequest(Object(Illuminate\\Http\\Request), Object(Closure), Array)
#18 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php(55): Illuminate\\Routing\\Middleware\\ThrottleRequests->handleRequestUsingNamedLimiter(Object(Illuminate\\Http\\Request), Object(Closure), 'api', Object(Closure))
#19 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Routing\\Middleware\\ThrottleRequests->handle(Object(Illuminate\\Http\\Request), Object(Closure), 'api')
#20 /mysite/vendor/statamic/cms/src/API/Middleware/Cache.php(26): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#21 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\API\\Middleware\\Cache->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#22 /mysite/vendor/statamic/cms/src/Http/Middleware/RequireStatamicPro.php(17): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#23 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\Http\\Middleware\\RequireStatamicPro->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#24 /mysite/vendor/statamic/cms/src/Http/Middleware/SwapExceptionHandler.php(19): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#25 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\Http\\Middleware\\SwapExceptionHandler->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#26 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#27 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Router.php(697): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#28 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Router.php(672): Illuminate\\Routing\\Router->runRouteWithinStack(Object(Illuminate\\Routing\\Route), Object(Illuminate\\Http\\Request))
#29 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Router.php(636): Illuminate\\Routing\\Router->runRoute(Object(Illuminate\\Http\\Request), Object(Illuminate\\Routing\\Route))
#30 /mysite/vendor/laravel/framework/src/Illuminate/Routing/Router.php(625): Illuminate\\Routing\\Router->dispatchToRoute(Object(Illuminate\\Http\\Request))
#31 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(166): Illuminate\\Routing\\Router->dispatch(Object(Illuminate\\Http\\Request))
#32 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(128): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}(Object(Illuminate\\Http\\Request))
#33 /mysite/vendor/statamic/cms/src/Http/Middleware/DisableFloc.php(18): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#34 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\Http\\Middleware\\DisableFloc->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#35 /mysite/vendor/statamic/cms/src/Http/Middleware/CheckMultisite.php(14): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#36 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\Http\\Middleware\\CheckMultisite->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#37 /mysite/vendor/statamic/cms/src/Http/Middleware/CheckComposerJsonScripts.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#38 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\Http\\Middleware\\CheckComposerJsonScripts->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#39 /mysite/vendor/statamic/cms/src/Http/Middleware/PoweredByHeader.php(19): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#40 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Statamic\\Http\\Middleware\\PoweredByHeader->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#41 /mysite/vendor/barryvdh/laravel-debugbar/src/Middleware/InjectDebugbar.php(60): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#42 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Barryvdh\\Debugbar\\Middleware\\InjectDebugbar->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#43 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#44 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php(31): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#45 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\ConvertEmptyStringsToNull->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#46 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#47 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TrimStrings.php(40): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#48 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\TrimStrings->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#49 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#50 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#51 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php(86): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#52 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\PreventRequestsDuringMaintenance->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#53 /mysite/vendor/fruitcake/laravel-cors/src/HandleCors.php(52): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#54 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Fruitcake\\Cors\\HandleCors->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#55 /mysite/vendor/fideloper/proxy/src/TrustProxies.php(57): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#56 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Fideloper\\Proxy\\TrustProxies->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#57 /mysite/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#58 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(141): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#59 /mysite/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(110): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter(Object(Illuminate\\Http\\Request))
#60 /mysite/public/index.php(52): Illuminate\\Foundation\\Http\\Kernel->handle(Object(Illuminate\\Http\\Request))
#61 /.composer/vendor/laravel/valet/server.php(235): require('/...')
#62 {main}

Versions

Statamic 3.2.10 Pro Laravel 8.62.0 PHP 7.4.24 statamic/ssg 0.8.0

Installation

Starter Kit using via CLI

Additional details

statamic new mysite statamic/starter-kit-starters-creek

jelleroorda commented 2 years ago

Hi @helloiamlukas

Are you registering your callback for Sentry based on the Laravel 8 exception handler?

Using the Sentry docs you are supposed to register it within a reportable callback inside the register method of your Exception handler.

helloiamlukas commented 2 years ago

@jelleroorda This problem is not only related to Sentry.

But yes, I also tried to registering the callback as in the Sentry docs (and the Laravel 8 exception handler). Doesn't work.

jelleroorda commented 2 years ago

I've checked why, and it turns out that there's a middleware that swaps out the exception handler for a ApiExceptionHandler. I'm not completely sure why that was done though. The only thing I can see is that it might have something to do with testing.

For now you can work around it by adding something like this in your AppServiceProvider@boot method:

        $this->app->resolving(\Illuminate\Contracts\Debug\ExceptionHandler::class, function($handler) {
            $handler->reportable(function(\Throwable $e) {
                dd('exception', $e);
            });
        });

It'll register your reportable handler for both exception handlers. Maybe @jasonvarga has a better solution or can explain why we need the ApiExceptionHandler?

helloiamlukas commented 2 years ago

@jelleroorda Good catch!

I would be also interested in why the exception handler has been swapped out. @jasonvarga please give us some insight 👨‍💻

helloiamlukas commented 2 years ago

Okay, I found something in the commit message https://github.com/statamic/cms/commit/9be827c449ee95cda697eafe9f3d39c470029a0e

Swap the exception handler in a middleware, since in a service provider we don't have access to the request and it would be too early to conditionally rebind.

Since it's in a middleware, it'll only use that exception handler (and thus handle the json 404 responses) when it actually hits a real route. Added a wildcard 404 route to the API like we have in the CP.

jelleroorda commented 2 years ago

From what I can see, maybe we could take this api rendering part and add it in the NotFoundHttpException@render method inside a manual check for whether it's an api route. Statamic::isApiRoute() wouldn't work since it doesn't handle the StatamicProAuthorizationException use case (it returns false if pro is disabled).

Let's wait for Jason for now 😄

jasonvarga commented 2 years ago

It was done so that we could control responses for CP and API errors. So yes if we can continue to use the appropriate responses without using our custom handlers... we can avoid swapping the handlers.