Custom exception handler

[iRedds]

Description
I offer functionality for global handling of custom exceptions.
An exception for handling will be defined through the interface

namespace CodeIgniter\Exceptions;

use CodeIgniter\HTTP\RequestInterface;
use CodeIgniter\HTTP\ResponseInterface;

interface CustomExceptionHandlerInterface
{
    public function renderResponse(RequestInterface $request): ResponseInterface;
}

An exception can be thrown from anywhere in the application.

Checklist:

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

[kenjis]

Thank you.

Please fix coding style,
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#php-style
and failed test.

[kenjis]

There was 1 failure:

1) CodeIgniter\Commands\CommandTest::testRoutesCommand
Failed asserting that '+---------+----------------------------+-----------------------------------------------------+\n
| Method  | Route                      | Handler                                             |\n
+---------+----------------------------+-----------------------------------------------------+\n
| GET     | /                          | \App\Controllers\Home::index                        |\n
| GET     | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| HEAD    | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| POST    | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| PUT     | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| DELETE  | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| OPTIONS | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| TRACE   | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| CONNECT | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
| CLI     | migrations/([^/]+)/([^/]+) | \CodeIgniter\Commands\MigrationsCommand::$1/$2      |\n
| CLI     | migrations/([^/]+)         | \CodeIgniter\Commands\MigrationsCommand::$1         |\n
| CLI     | migrations                 | \CodeIgniter\Commands\MigrationsCommand::index      |\n
| CLI     | ci(.*)                     | \CodeIgniter\CLI\CommandRunner::index/$1            |\n
| CLI     | exception                  | \Tests\Support\Controllers\Popcorn::customException |\n
+---------+----------------------------+-----------------------------------------------------+\n
\n
' contains "| testing".

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/CommandTest.php:114

https://github.com/codeigniter4/CodeIgniter4/runs/4250147232?check_suite_focus=true

[iRedds]

I modified the test by creating new class instances.
But the behavior of CommandTest::testRoutesCommand() seems to be wrong to me.

[kenjis]

@iRedds

But the behavior of CommandTest::testRoutesCommand() seems to be wrong to me.

What do you mean? If something wrong, please create an issue or a PR.

[kenjis]

@iRedds You mean changing another test makes CommandTest error?
If so, you're correct. It should not happen.

But in reality, there seems to be tests depending on other test results (global state).

[iRedds]

@iRedds You mean changing another test makes CommandTest error? If so, you're correct. It should not happen.

@kenjis Yes, that's what I meant.

In CodeIgniterTest, all tests use a closure, and the closure is excluded when generating the list of routes.
Therefore, up to this point, all tests passed without errors.

[lonnieezell]

I'm not convinced this is the best solution. This was something I had been thinking about implementing and hadn't gotten to it quite yet.

The first thing is that we already have a custom exception handler, and this solution is bypassing that and catching the exception before it makes it to the handler. The second is that it doesn't seem to allow custom handling of generic errors.

Here's where I was heading with my thinking of how to do it:

The exception handler that we already register simply:

1. catches all catchable errors
2. checks to see if a custom handler has been specified for the specific error/status code, or the type of exception.
3. If a custom handler has been found - run it.
4. If not run our normal functions, which is now split out into a handler.

Whether the handlers are defined in a config file, or in an overridden function, I'm not sure. Maybe a combination of the two?

[iRedds]

@lonnieezell Only exceptions that implement the CustomExceptionHandlerInterface interface are caught.
By your logic, any try..catch block in the application " is bypassing that and catching the exception before it makes it to the handler."

1. In fact, my PR is a replacement for the following code

//controller method
public function someMethod()
{
    try {
      //throws MyException
    } catch (MyException $e) {
      return //response
    }
}

public function someMethod2()
{
    try {
      //throws MyException
    } catch (MyException $e) {
      return //response
    }
}

by moving the try..catch block up one level.

2. My implementation allows to apply filter "after" to response.
3. All exceptions except custom ones will be handled as before.

At the expense of configuration files. I believe the config files should be config files. That is, they should not contain logic.
If you put it in a separate file, then it will have to be registered in the config file, because you cannot call it directly from the core of the framework. Since for some unthinkable reason, someone decided that it was necessary to give the opportunity to change the namespace of the application.
All of this creates an extra overhead, as with registering custom exception handlers separately.

Although initially I planned to make just a general exception handler, in which it would be possible to catch not only custom exceptions, but also exceptions of third-party libraries. It seems like it was in Laravel.

namespace App\Exceptions;

class Handler extends \Codeigniter\Exceptions\ExceptionHandler
{
    public function render(RequestInterface $request, \Exception $exception)
    {
        if (exception1) {
            // do something
        }
        
        if (exception2) {
            // do something
        }

    }
}