[DBAL-407] Refactor exceptions Created: 07/Jan/13 Updated: 27/Aug/13
|Component/s:||Drivers, Platforms, Schema Managers|
|Reporter:||Bart van den Burg||Assignee:||Benjamin Eberlei|
It's currently rather hard to figure out what went wrong when for example a DBALException was thrown. You have to actually match the message in it, or read the status code of the ->getPrevious() exception, which can be different for all drivers (as https://github.com/jackalope/jackalope-doctrine-dbal/issues/80 shows).
I'd suggest creating new exception classes for all situations and throwing them instead. If they extend the DBAL Exception and pass the message to it as it is right now, there will be no BC break.
If this were to be done, on which branch should this be applied?
|Comment by Christophe Coevoet [ 07/Jan/13 ]|
This should be done in the master branch.
Another solution, avoiding to create many classes, would be to use the exception code, which is always kept as 0 currently (the default value of the Exception class). You could have a code for each case (with constants in the DBALException class) and then checking $e->getCode() to identify what went wrong.
|Comment by Bart van den Burg [ 07/Jan/13 ]|
I'd prefer actual named exceptions. It makes catching them simpler. However, adding some code defined in DBAL would be an acceptable alternative.
|Comment by Christopher Davis [ 06/May/13 ]|
I would also prefer named exceptions. You're going to have a lot of problems providing the "code" value in DBALException in any case: SQLSTATE codes are alphanumeric, and will cause warnings/errors when creating new exception.
Besides we can get the SQL state code now:
The problem is that there are a lot of error codes defined in the ANSI SQL standard: http://www.postgresql.org/docs/9.2/static/errcodes-appendix.html
Maybe throwing an specific exception for each "class" of SQLSTATE codes? So if the error code from a PDO exception starts with 23, DBAL would throw `\Doctrine\DBAL\Exception\IntegrityConstraintViolationException`.
This also seems like the logic to handle throwing exceptions should be contained in the platforms as some implementations may differ. You could have a method in `AbstractPlatform` that takes care of the ANSI SQLSTATE error code classes and leave it up subclasses to deal with platform specific cases. Whenever `Connection` catches a `PDOException`, dispatch it to the platform to deal with.
|Comment by Matthieu Napoli [ 06/May/13 ]|
I have implemented a thing of that kind in a personal project (on top of Doctrine). It is really useful to be able to catch a ForeignKeyViolationException, and get with entity/field caused the problem (for that my EntityManager wrapper parse the exception message).
However, note that exception codes differ from DB engines. In my case, I did it quick and used MySQL error codes, but managing different RDBMS implies more work.
|Comment by Benjamin Eberlei [ 27/Aug/13 ]|
1. Introduce a list of constant status codes on \Doctrine\DBAL\DBALException for common errors: Duplicate Key, Integrity Constraint Violation (NOT NULL), table not found, ... (the three most common ones?)
2. Add a test to Doctrine DBAL Functional Tests that provoke the exception for Sqlite/MySQL/,.. to find out what their error code and error message is.
3. Write a simple helper function on every `\Doctrine\DBAL\Driver` and extend the interface, called "convertExceptionCode". Start with empty implementations return code 0 for every driver.
4. \Doctrine\DBAL\DBALException::driverExceptionDuringQuery() should be extended to accept the driver and convert the exception to an error code that is not the currently used one 0.
5. Introduce new exception types \Doctrine\DBAL\Exception\DuplicateKeyException, IntegrityConstraintViolationException, ...