Discussion: Exception Design

[kenjis]

The current exception classes are not neatly categorized, and when creating a new exception class, it is difficult to determine which exceptions to inherit and implement.

Also, when throwing exceptions, it is sometimes difficult to determine which exceptions to throw.

Click and see the class diagram:

source

In addition, the following questions arise.

• What is the difference between AlertError, CriticalError, and EmergencyError?
• What is the difference between an exception that extends Error and an exception that extends Exception?
• What is the difference between exceptions placed in CodeIgniter/Exceptions/ and exceptions placed in components such as CodeIgniter/Filters/?
• In what cases do we implement ExceptionInterface?
• What is the FrameworkException?
• Do we need CodeIgniter\Cache\Exceptions\ExceptionInterface?
• Shouldn't there be LogicException?

Another problem that has been found is that the exceptions thrown by database-related classes are disparate and not standardized. See #4331

It seems to me that we need to classify the exception classes in a way that is easy to understand, and also indicate what kind of exceptions we throw.

What are your thoughts?

[MGatner]

I think @paulbalandan spent quite a bit of time on this recently, and on framework exceptions in general. I agree that the chart makes it clear this is quite a mess and some standardization is in order. I expect that changing thrown types counts as a breaking change, so we may be limited to some degree on what we can patch in the short term, but let's plan for a full overhaul in version 5.

[kenjis]

Why DatabaseException extends Error?

• Throwable
  • Error
    • ArithmeticError
    • AssertionError
    • CompileError
    • TypeError
    • ValueError
    • UnhandledMatchError
    • FiberError
  • Exception

See https://www.php.net/manual/en/language.errors.php7.php#language.errors.php7.hierarchy

Error is not Exception. But DatabaseException seems to be a Runtime Exception.

[MGatner]

I have no idea why it was coded this way, perhaps @lonnieezell can chime in. I just spent some time reading up on best practices for this in PHP and here's what I've come to...

Errors were a hard failure point but at some point they were changed to be catchable. Because catch (Exception $e) is such a common practice PHP decided that an Error should not be included in that so defined a new superclass Throwable. This entire distinction appears to be a historical progression that we inherit today, and while there isn't community consensus on this most people stick to Exceptions and leave Errors for internal functions. This is also my recommendation.

Now: can we do this in a way that isn't massively breaking? My guess is that developers who are catching these currently are either using Throwable or DatabaseException, which will both still work. But anyone catching Error will not.

[kenjis]

most people stick to Exceptions and leave Errors for internal functions. This is also my recommendation.

I agree. I would like to remove throwing Error classes in the framework.

[kenjis]

I have created PR #8470.
The exception or parent class changes a little, but it does not seem to break existing apps in most cases.
What do you think? @codeigniter4/core-team

[kenjis]

I sent #8728
Please review.

[kenjis]

Closed by #8728