Move the logic of driver exception conversion into a separate interface

[morozov]

Q	A
Type	improvement
BC Break	yes

Currently, there is a need to have abstract driver classes because specific driver implementations need to share certain code. This could be avoided by moving the common bits of logic under separate interfaces and compose drivers instead. A similar approach is already used to separate the logic of the DB platforms and drivers and then combine them together under a DBAL connection.

New API namespace

The Driver\API namespace is created. It will contain the logic specific to the driver APIs which the database servers and clients use to communicate. Multiple DBAL drivers and PHP extensions (e.g. mysqli and pdo_mysql) may use the same API. The API may include such functions as handling of the error codes, building DSN, etc.

Changes in conversion of driver exceptions

1. The logic of conversion of the error codes to DBAL exeptions is moved under the ExceptionConverter interface.
2. The Driver::convertException() method has been replaced with getExceptionConverter(). This is a step towards converting the Driver interface from an implementation of the driver logic to an abstract factory of the driver components that could be composed together.

Changes in wrapping of driver exceptions

1. The logic of wrapping driver-level exceptions into DBAL ones is moved from DBALException to the wrapper Connection^†
2. The handleException*() methods in the wrapper connection have been reworked to convertException*() which return the exception instead of throwing it. The previous approach had certain downsides:
   1. Neither of PHPStan, Psalm and PhpStorm understand the never-returning methods out of the box. Their proper analysis requires additional annotations.
   2. Unlike the methods that return the value of a certain type and can be mocked by PHPUnit automatically based on reflection, the never-returning methods need to be mocked explicitly and cannot be made final (Deprecate PingableConnection in favor of handling lost connection #4119 (comment)).
   3. The exception is thrown one frame farther from its origin than it could be.

Behavior changes

1. DataAccessTest::testFetchAllWithMissingTypes has been removed. Besides not working with mysqli, pdo_sqlite and ibm_db2, the fact that it used to pass on PDO indicates an issue in the DBAL. The previous implementation used to catch (Throwable $e). As per the comments in PHP bug #79769, this error is not meant to be caught. By wrapping the error in a DBALException would make it less obvious that this was a logical error that had to be fixed, not caught.

† — The original idea was to have the wrapper-level exception converter implemented as a separate class and used by the connection but the connection itself also has the logic of handling exceptions. Implementing this in two separate classes would require a circular reference between the connection and the exception handler which might cause memory issues.

TODO:

• Document breaking changes
• Deprecate the corresponding APIs (Deprecate driver exception conversion APIs #4137)

[greg0ire]

the fact that it used to pass on PDO indicates an issue DBAL

Does your sentence maybe miss a word or two?

[morozov]

Yeah, it "indicates an issue in the DBAL".

[greg0ire]

Love the idea of using composition more 👍

[greg0ire]

And I meant to approve of course!