CodeIgniter4 icon indicating copy to clipboard operation
CodeIgniter4 copied to clipboard

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

Open paulbalandan opened this issue 2 years ago • 3 comments

Description This PR introduces a way to unregister the error and exception handlers originally registered in Services::exceptions()->initialize() by calling the unregister() method. Moreover, the initialize() method is deprecated in favor of the register() method simply for consistency with the unregister method.

Motivation PHPUnit 10 registers its own error handler. This may cause issues if the SUT is registering its own error handler (such as the issues encountered in #8069 ).

When PHPUnit’s test runner becomes aware (after it called set_error_handler() to register its error handler) that another error handler was registered then it immediately unregisters its error handler so that the previously registered error handler remains active. https://docs.phpunit.de/en/10.5/error-handling.html

To solve this, we have 2 possible solutions:

  1. Use the #[WithoutErrorHandler] attribute on each test case. This is problematic as this attribute is on a TARGET_METHOD so it needs to be applied to each test method.
  2. Let PHPUnit's own error handler do the job and unregister the SUT's own error handling.

This uses the 2nd approach by introducing the ExceptionHandlingTrait intended to be used in tests. The trait just provides shortcut access to the register/unregister methods.

This PR is intended to fix the failures in #8069 .

Checklist:

  • [x] Securely signed commits
  • [x] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [x] Unit testing, with >80% coverage
  • [ ] User guide updated
  • [x] Conforms to style guide

paulbalandan avatar Dec 28 '23 14:12 paulbalandan

1) CodeIgniter\Debug\ExceptionsTest::testUnregisteringUsesPhpUnitsOwnHandler
PHPUnit\Framework\Exception: PHP Fatal error:  Uncaught Error: Attempt to modify property "aliases" on null in /home/runner/work/CodeIgniter4/CodeIgniter4/tests/_support/Config/Filters.php:22
Stack trace:
#0 Standard input code(1177): require_once()
#1 {main}
  thrown in /home/runner/work/CodeIgniter4/CodeIgniter4/tests/_support/Config/Filters.php on line 22

https://github.com/codeigniter4/CodeIgniter4/actions/runs/7348347348/job/20006326903?pr=8380

kenjis avatar Dec 29 '23 01:12 kenjis

The weird thing is that this error comes out when running vendor/bin/phpunit --group Others but not on vendor/bin/phpunit --filter ExceptionsTest

paulbalandan avatar Dec 29 '23 05:12 paulbalandan

$ vendor/bin/phpunit tests/system/Debug/ExceptionsTest.php 
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.13
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/phpunit.xml

.........                                                                                                       9 / 9 (100%)

Time: 00:00.501, Memory: 16.00 MB

OK (9 tests, 37 assertions)
$ vendor/bin/phpunit tests/system/Debug/
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.13
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/phpunit.xml

..................E....................                                                                       39 / 39 (100%)

Time: 00:05.049, Memory: 20.00 MB

There was 1 error:

1) CodeIgniter\Debug\ExceptionsTest::testUnregisteringUsesPhpUnitsOwnHandler
PHPUnit\Framework\Exception: ERROR: 
PHP Fatal error:  Uncaught TypeError: CodeIgniter\CLI\CLI::write(): Argument #1 ($text) must be of type string, null given, called in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/app/Views/errors/cli/error_404.php on line 6 and defined in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/system/CLI/CLI.php:454
Stack trace:
#0 /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/app/Views/errors/cli/error_404.php(6): CodeIgniter\CLI\CLI::write(NULL)
#1 Standard input code(701): require_once('/Users/kenji/wo...')
#2 {main}
  thrown in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/system/CLI/CLI.php on line 454

ERRORS!
Tests: 39, Assertions: 228, Errors: 1.
$ vendor/bin/phpunit --group=Others
PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.13
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/phpunit.xml

.........................................................................................................  105 / 5112 (  2%)
...
........................................................................                                  5112 / 5112 (100%)

...


Time: 01:27.991, Memory: 126.50 MB

There was 1 error:

1) CodeIgniter\Debug\ExceptionsTest::testUnregisteringUsesPhpUnitsOwnHandler
PHPUnit\Framework\Exception: PHP Fatal error:  Uncaught Error: Attempt to modify property "aliases" on null in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/tests/_support/Config/Filters.php:22
Stack trace:
#0 Standard input code(1144): require_once()
#1 {main}
  thrown in /Users/kenji/work/codeigniter/official/CodeIgniter4-4.5/tests/_support/Config/Filters.php on line 22

ERRORS!
Tests: 5112, Assertions: 8649, Errors: 1.

kenjis avatar Dec 29 '23 09:12 kenjis

@paulbalandan The following patch makes CodeIgniterTest pass. (You can see the errors: https://github.com/codeigniter4/CodeIgniter4/pull/8069#issuecomment-1963176259) So it is true that the CI's Error Handler does something wrong.

But I don't understand:

  • why on Wiindows there are no errors
  • why only PHP 8.3 causes the errors
--- a/system/Debug/Exceptions.php
+++ b/system/Debug/Exceptions.php
@@ -109,7 +109,7 @@ class Exceptions
     public function initialize()
     {
         set_exception_handler([$this, 'exceptionHandler']);
-        set_error_handler([$this, 'errorHandler']);
+        //set_error_handler([$this, 'errorHandler']);
         register_shutdown_function([$this, 'shutdownHandler']);
     }
 

kenjis avatar Feb 26 '24 02:02 kenjis