symfony: [Framework bundle] Cleanup error and exception handlers on Kernel shutdown

Symfony version(s) affected

7.0.0

Description

PHPUnit 11 checks for any leftovers in error handlers https://github.com/sebastianbergmann/phpunit/pull/5619

How to reproduce

Run FrameworkBundle->boot() in phpunit test.

https://github.com/symfony/symfony/blob/815df931dca4663a6375e205b0fdf6e546ebfa75/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php#L98

It results in Test code or tested code did not remove its own exception handlers thrown by phpunit.

Possible Solution

Introduce FrameworkBundle::shutdown() that would cleanup error handler set in boot(). IMO it’s reasonable to have some cleanup in shutdown() of things initiated in boot().

Additional Context

No response

About this issue

  • Original URL
  • State: open
  • Created 5 months ago
  • Reactions: 15
  • Comments: 16 (11 by maintainers)

Commits related to this issue

Most upvoted comments

A different workaround, seeing as the Symfony/Errorhandler is a singleton and will only register once, is to register the error handler before phpunit memorizes all the existing error handlers. This way the error handler Symfony register’s is not new.

Create (or modify) /tests/bootstrap.php:

<?php
declare(strict_types=1);

use Symfony\Component\ErrorHandler\ErrorHandler;

ErrorHandler::register(null, false);

and in phpunit.xml

<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.0/phpunit.xsd"
         bootstrap="tests/bootstrap.php">
...
</phpunit>

@frankdekker is right, the error handler registered by FrameworkBundle would not replace the active error handler. PHPUnit already registers its own error handler and that one is still active at the end of the test. So we’re good here.

The problem which PHPUnit reports is about the exception handler. Yes, error handler and exception handler are two different things. And yes, it’s super confusing that Symfony has a method called ErrorHandler::register() that registers an error handler and and exception handler. 😓

PHPUnit on the other hand does not register an exception handler and even if it did, that handler would be wrapped by ErrorHandler::register() which PHPUnit would still rightfully complain about.

But. ErrorHandler won’t replace the exception handler if ErrorHandleralready is the current exception handler. This means, we can simplify @frankdekker’s bootstrap script like this:

<?php

declare(strict_types=1);

use Symfony\Component\ErrorHandler\ErrorHandler;

require dirname(__DIR__).'/vendor/autoload.php';

set_exception_handler([new ErrorHandler(), 'handleException']);

That being said, this feels like a hack. Maybe we should add a way to opt out of the exception handler. In situations where the kernel is executed within a huge try/catch block (which a PHPUnit test basically is), that exception handler does not serve any purpose anyway.

Same issue “Test code or tested code did not remove its own exception handlers” here with

  • PHP 8.2.15
  • Symfony 6.4.3
  • phpunit 11

What if we stopped registering error handlers, exception handlers or shutdown functions of any kind in FrameworkBundle?

We do need this stuff way before the framework is booted, so whatever FrameworkBundle registers here, it does it way too late. This applies especially to exception handlers: They are meant to handle uncaught exceptions. The kernel however runs in a big try/catch, so at the time FrameworkBundle is booted, the need for an exception handler is more or less over.

In modern Symfony applications, the error handling should be set up by the runtime already. All that FrameworkBundle does is connecting the logger properly. The logic that registers an error handler or exception handler is usually dormant and causes trouble because PHPUnit tests run outside of the runtime.

I had the same problem with Symfony 6.4 and PHPUnit 11. I temporarily solved it by calling this method in the tearDown of my base test case (which extends from KernelTestCase or WebTestCase):

protected function restoreExceptionHandler(): void
{
    while (true) {
        $previousHandler = set_exception_handler(static fn() => null);

        restore_exception_handler();

        if ($previousHandler === null) {
            break;
        }

        restore_exception_handler();
    }
}

The tearDown looks like this:

protected function tearDown(): void
{
    parent::tearDown();

    $this->restoreExceptionHandler();
}

It is based on what Laravel did on their testsuite: https://github.com/laravel/framework/pull/49622/files

Maybe the solution is to call a similar method in the tearDown of KernelTestCase and WebTestCase?

This issue is a blocker for PHPUnit 11 in any Symfony project that uses KernelTestCase or WebTestCase. Given that PHPUnit comes with its own error handler, maybe the solution would be to opt out of Symfony’s own error handler when booting the kernel for functional tests?

Recently created a new Project with the Lumen Framework and wrote Tests for it and got this Warnings:

* Test code or tested code did not remove its own error handlers * Test code or tested code did not remove its own exception handlers

  • PHP 8.3.3
  • PhpUnit 11.0.6
  • Lumen 10.0.3

To overcome this Problem I needed to put this in the TestCase Class:


    protected function restoreExceptionHandler(): void
    {
        while (true) {
            $previousHandler = set_exception_handler(static fn() => null);
            restore_exception_handler();
            if ($previousHandler === null) {
                break;
            }
            restore_exception_handler();
        }
    }

    protected function restoreErrorHandler(): void
    {
        while (true) {
            $previousHandler = set_error_handler(static fn() => null);
            restore_error_handler();
            $isPhpUnitErrorHandler = ($previousHandler instanceof \PHPUnit\Runner\ErrorHandler);
            if ($previousHandler === null || $isPhpUnitErrorHandler) {
                break;
            }
            restore_error_handler();
        }
    }

    protected function tearDown(): void
    {
        parent::tearDown();
        $this->restoreErrorHandler();
        $this->restoreExceptionHandler();
    }

I hope this helps others, too.

Will this issue be resolved by symfony or is it expected to add @derrabus solution to every project?

This will be resolved by whomever resolves it. Could be you?

Will this issue be resolved by symfony or is it expected to add @derrabus solution to every project?

that uses KernelTestCase or WebTestCase

this is not a root cause. I’d correct that to:

This issue is a blocker for PHPUnit 11 in any Symfony project that uses Kernel in tests.

E.g. for me the test maintenance is much easier when I’m not relying on SF testcases so I can extend my own leaner testcase. I use Kernel via composition in tests.

So the root cause is that there’s no way to cleanup after Kernel, not some test case.