kernel.exception is not triggered when (e.g.) new relic registers an error handler #25827

linaori · 2018-01-18T08:39:03Z

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	no
RFC?	no
Symfony version	3.4 (probably lower as well)

After @nicolas-grekas dug in this for me on slack, it turns out that the Symfony error handler is not registered because new relic already registers on.

Related issue seems to be an old one: https://discuss.newrelic.com/t/how-to-disable-newrelic-set-exception-handler/35544/16

Snippet of our discussion on Slack:

in dev, Debug::enable() always installs Debug::ErrorHandler()
so that DebugHandlersListener is able to register the required logic
in prod, ErrorHandler::register() is called in FrameworkBundle::boot()
but with $replace=false as second argument
so that with new relic, no registration happen
making DebugHandlersListener a no-op
I'm now wondering if ErrorHandler's line L135 shouldn't be unconditional
the decoration is seamless I think

It seems that exceptions are still caught, as long as they are custom exceptions. Fatal errors (and possibly core exceptions) are not caught.

PHP Fatal error:  Uncaught Symfony\\Component\\Debug\\Exception\\FatalThrowableError: Cannot use object of type Hostnet\\App\\Component\\Security\\Authentication\\Oauth\\OauthCredentials as array in /home/prod/mijn/12.17.7____0/src/App/EventListener/Security/StoreLoginLog.php:86\nStack trace:\n#0 /home/prod/mijn/12.17.7____0/vendor/symfony/event-dispatcher/EventDispatcher.php(212): Hostnet\\App\\EventListener\\Security\\StoreLoginLog->onAuthenticationFailure(Object(Symfony\\Component\\Security\\Core\\Event\\AuthenticationFailureEvent), 'security.authen...', Object(Symfony\\Component\\EventDispatcher\\ContainerAwareEventDispatcher))\n#1 /home/prod/mijn/12.17.7____0/vendor/symfony/event-dispatcher/EventDispatcher.php(44): Symfony\\Component\\EventDispatcher\\EventDispatcher->doDispatch(Array, 'security.authen...', Object(Symfony\\Component\\Security\\Core\\Event\\AuthenticationFailureEvent))\n#2 /home/prod/mijn/12.17.7____0/vendor/symfony/security/Core/Authentication/AuthenticationProviderManager.php(107): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispa in /home/prod/mijn/12.17.7____0/src/App/EventListener/Security/StoreLoginLog.php on line 86, referer: https://accounts.google.com/...
PHP Stack trace:, referer: https://accounts.google.com/...
PHP   1. Symfony\\Component\\Debug\\ErrorHandler->handleException() /home/prod/mijn/12.17.7____0/vendor/symfony/debug/ErrorHandler.php:0, referer: https://accounts.google.com/...
PHP   2. newrelic_exception_handler() /home/prod/mijn/12.17.7____0/vendor/symfony/debug/ErrorHandler.php:538, referer: https://accounts.google.com/signin/oauth/...

The class logging this (similar to the class that's redirecting it), are registered on the kernel.exception event (which is no longer triggered):

// ...
final class UncaughtExceptionLogger implements EventSubscriberInterface
{
    private $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    public function onKernelException(GetResponseForExceptionEvent $event): void
    {
        $e = $event->getException()
        $this->logger->error('Uncaught PHP Exception: ' . \get_class($e), ['exception' => $e]);
    }

    public static function getSubscribedEvents(): array
    {
        return [KernelEvents::EXCEPTION => ['onKernelException', -16]];
    }
}

In result with new relic:

app.php: white 500 webserver error page, kernel.exception is not triggered
app_dev.php: working as intended
apache error logging work fine
new relic error logging works fine

The text was updated successfully, but these errors were encountered:

…l with fatal errors (nicolas-grekas) This PR was merged into the 2.7 branch. Discussion ---------- [Debug] Always decorate existing exception handlers to deal with fatal errors | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25827 | License | MIT | Doc PR | - Decorating the exception is seamless, let's always do it and fix handling of fatal errors. Related to #25408 also. Commits ------- 205d7ae [Debug] Always decorate existing exception handlers to deal with fatal errors

linaori · 2018-01-18T12:16:54Z

@fabpot can you reopen this issue please? It's not fixed yet 😢

…he existing exception handler (nicolas-grekas) This PR was merged into the 2.7 branch. Discussion ---------- [HttpKernel] DebugHandlersListener should always replace the existing exception handler | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25827 | License | MIT | Doc PR | - The current logic is inconsistent because replacing or not depends whether an ExceptionHandler is registered or not. Embeds tests for the previous PR on the same topic, Debug component's side. Commits ------- a4ddcc2 [HttpKernel] DebugHandlersListener should always replace the existing exception handler

nicolas-grekas mentioned this issue Jan 18, 2018

[Debug] Always decorate existing exception handlers to deal with fatal errors #25829

Merged

fabpot closed this as completed Jan 18, 2018

xabbuh reopened this Jan 18, 2018

nicolas-grekas mentioned this issue Jan 18, 2018

[HttpKernel] DebugHandlersListener should always replace the existing exception handler #25835

Merged

nicolas-grekas closed this as completed Jan 18, 2018

sroze mentioned this issue Jan 30, 2018

[Debug] Memory leak in Debug\ErrorHandler #25965

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kernel.exception is not triggered when (e.g.) new relic registers an error handler #25827

kernel.exception is not triggered when (e.g.) new relic registers an error handler #25827

linaori commented Jan 18, 2018 •

edited

Loading

linaori commented Jan 18, 2018

kernel.exception is not triggered when (e.g.) new relic registers an error handler #25827

kernel.exception is not triggered when (e.g.) new relic registers an error handler #25827

Comments

linaori commented Jan 18, 2018 • edited Loading

linaori commented Jan 18, 2018

linaori commented Jan 18, 2018 •

edited

Loading