sentry-php
sentry-php copied to clipboard
Add a way to cleanup error and exception handlers
Problem Statement
PHPUnit 11 checks for any leftovers in error handlers https://github.com/sebastianbergmann/phpunit/pull/5619
Solution Brainstorm
When \Sentry\ErrorHandler::registerOnceErrorHandler() is called, error handler is registered.
It would be nice to add degisterOnceErrorHandler(). Since the error handler is kept in ErrorHandler's state, it would be easy to deregister it.
That makes sense, let me try to use PHPUnit 11 in the SDK tests and see what happens 😄
Can I ask why you would want this?
Sentry shouldn't be active when running tests ideally and our own test suite is not going to be ready for PHPUnit 11 for some time probably. If you do load Sentry in your tests isn't it better to prevent loading the (fatal) error handler integrations so they don't register in the first place instead of de-registering them afterwards which I'm assuming needs to be done manually by the user in their own code anyway?
There is no method to de-register a specific error handler so by the time a degisterOnceErrorHandler is called other handler could have been registered causing us to remove other handlers in the process.
When doing full app integration tests I want to simulate the same environment as is in prod in order to test exactly the same thing that runs there.
And how would you have setup the degisterOnceErrorHandler in your test suite if it existed?
Is not registering the error handler integrations not an good compromise here? I'm assuming your integration tests don't test for unhandled or fatal errors caught by Sentry? Or do they?
Is not registering the error handler integrations not an good compromise here? I'm assuming your integration tests don't test for unhandled or fatal errors caught by Sentry? Or do they?
Sure but then my tests skip some part of code that get's executed in prod and therefore do not cover it.
Currently my workaround in teardown looks similar to this:
$res = [];
while (true) {
$previousHandler = set_exception_handler(static fn () => null);
restore_exception_handler();
if ($previousHandler instanceof Closure) {
$reflection = new ReflectionFunction($previousHandler);
if (
$reflection->getClosureThis() instanceof \Sentry\ErrorHandler
&& $reflection->getName() === 'handleException'
) {
restore_exception_handler();
continue;
}
}
if ($previousHandler === null) {
break;
}
$res[] = $previousHandler;
restore_exception_handler();
}
$res = array_reverse($res);
foreach ($res as $handler) {
set_exception_handler($handler);
}
as imperfect as it may be, it allowed me to run my tests with phpunit 11 without compromising coverage.
@cleptric what is the sentry's statement on this?
What @stayallive said.