Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ExceptionHandlingTrait and Exceptions::(un)register #8380

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/Views/errors/cli/error_404.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
<?php

use CodeIgniter\CLI\CLI;
use CodeIgniter\Debug\Exceptions;

if (Exceptions::isUsingPhpUnitErrorHandlingInSeparateProcess()) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't still understand what's happening.
Can you explain what is the exact cause of the errors?
This hack is very bad, because the view file should not know about PHPUnit process isolation.
Can't we avoid to do like this?


CLI::error('ERROR: ' . $code);
CLI::write($message);
Expand Down
5 changes: 5 additions & 0 deletions app/Views/errors/cli/error_exception.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
<?php

use CodeIgniter\CLI\CLI;
use CodeIgniter\Debug\Exceptions;

if (Exceptions::isUsingPhpUnitErrorHandlingInSeparateProcess()) {
return;
}

// The main Exception
CLI::write('[' . get_class($exception) . ']', 'light_gray', 'red');
Expand Down
2 changes: 1 addition & 1 deletion system/CodeIgniter.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public function initialize()
$this->bootstrapEnvironment();

// Setup Exception Handling
Services::exceptions()->initialize();
Services::exceptions()->register();

// Run this check for manual installations
if (! is_file(COMPOSER_PATH)) {
Expand Down
41 changes: 41 additions & 0 deletions system/Debug/Exceptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,35 @@ public function __construct(ExceptionsConfig $config)
*
* @codeCoverageIgnore
*
* @deprecated 4.5.0 Use `register()` instead
*
* @return void
*/
public function initialize()
{
$this->register();
}

/**
* Responsible for registering the error handler, exception handler,
* and shutdown handler.
*/
public function register(): void
{
set_exception_handler([$this, 'exceptionHandler']);
set_error_handler([$this, 'errorHandler']);
register_shutdown_function([$this, 'shutdownHandler']);
}

/**
* Unregisters the exception handler and error handler.
*/
public function unregister(): void
{
restore_exception_handler();
restore_error_handler();
}

/**
* Catches any uncaught errors and exceptions, including most Fatal errors
* (Yay PHP7!). Will log the error, display it if display_errors is on,
Expand Down Expand Up @@ -594,6 +614,27 @@ public static function highlightFile(string $file, int $lineNumber, int $lines =
return '<pre><code>' . $out . '</code></pre>';
}

/**
* When PHPUnit 9.x is running on a separate process, it registers its
* simple error handler through the function '__phpunit_error_handler'.
*
* This checks whether the test is running in a separate process without
* creating a direct dependency on PHPUnit.
*
* @todo Check if the same function is used on PHPUnit 10.x
*
* @internal
*/
public static function isUsingPhpUnitErrorHandlingInSeparateProcess(): bool
{
try {
// @phpstan-ignore-next-line
return set_error_handler(null) === '__phpunit_error_handler';
} finally {
restore_error_handler();
}
}

private static function renderBacktrace(array $backtrace): string
{
$backtraces = [];
Expand Down
33 changes: 33 additions & 0 deletions system/Test/ExceptionHandlingTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Test;

use Config\Services;

/**
* Provides utilities for registering and unregistering
* of the exception and error handlers.
*/
trait ExceptionHandlingTrait
{
protected function enableExceptionHandling(): void
{
Services::exceptions()->register();
}

protected function disableExceptionHandling(): void
{
Services::exceptions()->unregister();
}
}
5 changes: 5 additions & 0 deletions tests/_support/Config/Filters.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@

namespace Tests\Support\Config\Filters;

use CodeIgniter\Debug\Exceptions;
use Tests\Support\Filters\Customfilter;
use Tests\Support\Filters\RedirectFilter;

if (Exceptions::isUsingPhpUnitErrorHandlingInSeparateProcess()) {
return;
}

/**
* @psalm-suppress UndefinedGlobalVariable
*/
Expand Down
24 changes: 17 additions & 7 deletions tests/system/Debug/ExceptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use CodeIgniter\Test\ReflectionHelper;
use Config\Exceptions as ExceptionsConfig;
use ErrorException;
use PHPUnit\Framework\Exception;
use RuntimeException;

/**
Expand Down Expand Up @@ -65,7 +66,7 @@ public function testDeprecationsOnPhp81DoNotThrow(): void
$config->deprecationLogLevel = 'error';

$this->exception = new Exceptions($config);
$this->exception->initialize();
$this->exception->register();

// this is only needed for IDEs not to complain that strlen does not accept explicit null
$maybeNull = PHP_VERSION_ID >= 80100 ? null : 'random string';
Expand All @@ -74,11 +75,10 @@ public function testDeprecationsOnPhp81DoNotThrow(): void
// We test DEPRECATED error, so cannot set `declare(strict_types=1)` in this file.
strlen($maybeNull);
$this->assertLogContains('error', '[DEPRECATED] strlen(): ');
} catch (ErrorException $e) {
} catch (ErrorException) {
$this->fail('The catch block should not be reached.');
} finally {
restore_error_handler();
restore_exception_handler();
$this->exception->unregister();
}
}

Expand All @@ -90,13 +90,12 @@ public function testSuppressedDeprecationsAreLogged(): void
$config->deprecationLogLevel = 'error';

$this->exception = new Exceptions($config);
$this->exception->initialize();
$this->exception->register();

@trigger_error('Hello! I am a deprecation!', E_USER_DEPRECATED);
$this->assertLogContains('error', '[DEPRECATED] Hello! I am a deprecation!');

restore_error_handler();
restore_exception_handler();
$this->exception->unregister();
}

public function testDetermineViews(): void
Expand Down Expand Up @@ -229,4 +228,15 @@ public function testMaskSensitiveDataTraceDataKey(): void

$this->assertSame('/var/www/CodeIgniter4/app/Controllers/Home.php', $newTrace[0]['file']);
}

/**
* @runInSeparateProcess
*/
public function testUnregisteringUsesPhpUnitsOwnHandler(): void
{
$this->exception->unregister();

$this->expectException(Exception::class); // PHPUnit\Framework\Exception
@trigger_error('Hello', E_USER_ERROR);
}
}
Loading