Skip to content

Commit e3646e9

Browse files
sebastianbergmannsebastianbergmann
committed
Merge branch '13.2'
* 13.2: Do not record an issue twice when previous error handler delegates it back Update ChangeLog Guard against recursive previous error handler forwarding
2 parents b7328dc + c7925cc commit e3646e9

4 files changed

Lines changed: 149 additions & 7 deletions

File tree

‎src/Runner/ErrorHandler.php‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ final class ErrorHandler
7575
private bool $enabled = false;
7676
private ?int $originalErrorReportingLevel = null;
7777

78+
/**
79+
* @var ?array{int, string, string, int}
80+
*/
81+
private ?array $forwardedError = null;
82+
7883
/**
7984
* @var ?callable
8085
*/
@@ -141,6 +146,15 @@ private function __construct(bool $identifyIssueTrigger)
141146
*/
142147
public function __invoke(int $errorNumber, string $errorString, string $errorFile, int $errorLine): bool
143148
{
149+
/**
150+
* A previously registered error handler may delegate an error that is being
151+
* forwarded to it back to this error handler: the issue must only be recorded
152+
* by the invocation that forwards the error, not by the delegating invocation.
153+
*/
154+
if ($this->forwardedError === [$errorNumber, $errorString, $errorFile, $errorLine]) {
155+
return false;
156+
}
157+
144158
$suppressed = (error_reporting() & ~self::INSUPPRESSIBLE_LEVELS) === 0;
145159

146160
if ($suppressed && $this->excludeList->isExcluded($errorFile)) {
@@ -288,6 +302,15 @@ public function __invoke(int $errorNumber, string $errorString, string $errorFil
288302

289303
public function handleNonTestCaseIssue(int $errorNumber, string $errorString, string $errorFile, int $errorLine): true
290304
{
305+
/**
306+
* A previously registered error handler may delegate an error that is being
307+
* forwarded to it back to this error handler: the issue must only be recorded
308+
* by the invocation that forwards the error, not by the delegating invocation.
309+
*/
310+
if ($this->forwardedError === [$errorNumber, $errorString, $errorFile, $errorLine]) {
311+
return true;
312+
}
313+
291314
$suppressed = (error_reporting() & ~self::INSUPPRESSIBLE_LEVELS) === 0;
292315

293316
if ($suppressed && $this->excludeList->isExcluded($errorFile)) {
@@ -302,9 +325,7 @@ public function handleNonTestCaseIssue(int $errorNumber, string $errorString, st
302325

303326
if ($errorString === '' || $errorFile === '' || $errorLine < 1) {
304327
// @codeCoverageIgnoreStart
305-
if ($this->previousNonTestCaseErrorHandler !== null) {
306-
($this->previousNonTestCaseErrorHandler)($errorNumber, $errorString, $errorFile, $errorLine);
307-
}
328+
$this->forwardToPreviousNonTestCaseErrorHandler($errorNumber, $errorString, $errorFile, $errorLine);
308329

309330
return true;
310331
// @codeCoverageIgnoreEnd
@@ -317,9 +338,7 @@ public function handleNonTestCaseIssue(int $errorNumber, string $errorString, st
317338
*
318339
* @see https://github.com/sebastianbergmann/phpunit/issues/6817
319340
*/
320-
if ($this->previousNonTestCaseErrorHandler !== null) {
321-
($this->previousNonTestCaseErrorHandler)($errorNumber, $errorString, $errorFile, $errorLine);
322-
}
341+
$this->forwardToPreviousNonTestCaseErrorHandler($errorNumber, $errorString, $errorFile, $errorLine);
323342

324343
/**
325344
* E_STRICT is deprecated since PHP 8.4.
@@ -1035,7 +1054,7 @@ private function deprecationIgnoredByFilter(string $message, string $file, int $
10351054

10361055
private function forwardToPreviousErrorHandler(int $errorNumber, string $errorString, string $errorFile, int $errorLine): bool
10371056
{
1038-
if ($this->previousErrorHandler === null) {
1057+
if ($this->previousErrorHandler === null || $this->forwardedError !== null) {
10391058
return false;
10401059
}
10411060

@@ -1059,12 +1078,31 @@ private function forwardToPreviousErrorHandler(int $errorNumber, string $errorSt
10591078
$restoreRequired = true;
10601079
}
10611080

1081+
$this->forwardedError = [$errorNumber, $errorString, $errorFile, $errorLine];
1082+
10621083
try {
10631084
return (bool) ($this->previousErrorHandler)($errorNumber, $errorString, $errorFile, $errorLine);
10641085
} finally {
1086+
$this->forwardedError = null;
1087+
10651088
if ($restoreRequired) {
10661089
error_reporting($errorReportingLevel);
10671090
}
10681091
}
10691092
}
1093+
1094+
private function forwardToPreviousNonTestCaseErrorHandler(int $errorNumber, string $errorString, string $errorFile, int $errorLine): void
1095+
{
1096+
if ($this->previousNonTestCaseErrorHandler === null || $this->forwardedError !== null) {
1097+
return;
1098+
}
1099+
1100+
$this->forwardedError = [$errorNumber, $errorString, $errorFile, $errorLine];
1101+
1102+
try {
1103+
($this->previousNonTestCaseErrorHandler)($errorNumber, $errorString, $errorFile, $errorLine);
1104+
} finally {
1105+
$this->forwardedError = null;
1106+
}
1107+
}
10701108
}

‎tests/end-to-end/error-handler/previous-error-handler-recursion-events.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
PHPUnit records an issue only once when previous error handler delegates it back
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$_SERVER['argv'][] = '--do-not-cache-result';
6+
$_SERVER['argv'][] = '--no-configuration';
7+
$_SERVER['argv'][] = '--debug';
8+
$_SERVER['argv'][] = __DIR__ . '/_files/PreviousErrorHandlerTest.php';
9+
10+
require __DIR__ . '/../../bootstrap.php';
11+
12+
use PHPUnit\Runner\ErrorHandler;
13+
14+
set_error_handler(static function (int $errno, string $errstr, string $errfile, int $errline): bool {
15+
return ErrorHandler::instance()->__invoke($errno, $errstr, $errfile, $errline);
16+
});
17+
18+
(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
19+
--EXPECTF--
20+
PHPUnit Started (PHPUnit %s using %s)
21+
Test Runner Configured
22+
Event Facade Sealed
23+
Test Suite Loaded (1 test)
24+
Test Runner Started
25+
Test Suite Sorted
26+
Test Runner Execution Started (1 test)
27+
Test Suite Started (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest, 1 test)
28+
Test Preparation Started (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest::testUserNotice)
29+
Test Prepared (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest::testUserNotice)
30+
Test Triggered Notice (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest::testUserNotice) in %sPreviousErrorHandlerTest.php:%d
31+
notice from test
32+
Test Passed (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest::testUserNotice)
33+
Test Finished (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest::testUserNotice)
34+
Test Suite Finished (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerTest, 1 test)
35+
Test Runner Execution Finished
36+
Test Runner Finished
37+
PHPUnit Finished (Shell Exit Code: 0)

‎tests/end-to-end/error-handler/previous-error-handler-recursion-file-scope.phpt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
PHPUnit records a file-scope issue only once when previous error handler delegates it back
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$_SERVER['argv'][] = '--do-not-cache-result';
6+
$_SERVER['argv'][] = '--no-configuration';
7+
$_SERVER['argv'][] = '--debug';
8+
$_SERVER['argv'][] = __DIR__ . '/_files/PreviousErrorHandlerFileScopeTest.php';
9+
10+
require __DIR__ . '/../../bootstrap.php';
11+
12+
use PHPUnit\Runner\ErrorHandler;
13+
14+
set_error_handler(static function (int $errno, string $errstr, string $errfile, int $errline): bool {
15+
ErrorHandler::instance()->handleNonTestCaseIssue($errno, $errstr, $errfile, $errline);
16+
17+
return true;
18+
});
19+
20+
(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
21+
--EXPECTF--
22+
PHPUnit Started (PHPUnit %s using %s)
23+
Test Runner Configured
24+
Event Facade Sealed
25+
Test Runner Triggered Notice () in %sPreviousErrorHandlerFileScopeTest.php:%d
26+
file scope user notice
27+
Test Suite Loaded (1 test)
28+
Test Runner Started
29+
Test Suite Sorted
30+
Test Runner Execution Started (1 test)
31+
Test Suite Started (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerFileScopeTest, 1 test)
32+
Test Preparation Started (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerFileScopeTest::testSuccess)
33+
Test Prepared (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerFileScopeTest::testSuccess)
34+
Test Passed (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerFileScopeTest::testSuccess)
35+
Test Finished (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerFileScopeTest::testSuccess)
36+
Test Suite Finished (PHPUnit\TestFixture\ErrorHandler\PreviousErrorHandlerFileScopeTest, 1 test)
37+
Test Runner Execution Finished
38+
Test Runner Finished
39+
PHPUnit Finished (Shell Exit Code: 0)

‎tests/end-to-end/error-handler/previous-error-handler-recursion.phpt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
PHPUnit does not enter an infinite loop when previous error handler calls it back
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$_SERVER['argv'][] = '--do-not-cache-result';
6+
$_SERVER['argv'][] = '--no-configuration';
7+
$_SERVER['argv'][] = __DIR__ . '/_files/PreviousErrorHandlerTest.php';
8+
9+
require __DIR__ . '/../../bootstrap.php';
10+
11+
use PHPUnit\Runner\ErrorHandler;
12+
13+
set_error_handler(static function (int $errno, string $errstr, string $errfile, int $errline): bool {
14+
return ErrorHandler::instance()->__invoke($errno, $errstr, $errfile, $errline);
15+
});
16+
17+
(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
18+
--EXPECTF--
19+
PHPUnit %s by Sebastian Bergmann and contributors.
20+
21+
Runtime: PHP %s
22+
23+
N 1 / 1 (100%)
24+
25+
Time: %s, Memory: %s
26+
27+
OK, but there were issues!
28+
Tests: 1, Assertions: 1, Notices: 1.

0 commit comments

Comments
 (0)