Catch clause is not understood

[ghost]

The catch clause in the ConnectionFactory is poorly understood.

private function getDatabasePlatform(Connection $connection)
    {
        try {
            return $connection->getDatabasePlatform();
        } catch (DBALException $driverException) {
            if ($driverException instanceof DriverException) {
                throw new DBALException(
                    'An exception occured while establishing a connection to figure out your platform version.' . PHP_EOL .
                    "You can circumvent this by setting a 'server_version' configuration value" . PHP_EOL . PHP_EOL .
                    'For further information have a look at:' . PHP_EOL .
                    'https://github.com/doctrine/DoctrineBundle/issues/673',
                    0,
                    $driverException
                );
            }
            throw $driverException;
        }
    }

Catch the DriverException directly istead.

[alcaeus]

Everything happens for a reason, in this case a missing class:
#677 (comment). However, this is a relict from past days. With DBAL 2.5 being the minimum supported version this can be changed. If you want to fix this you can send a PR to the 1.10.x branch. 👍

[alcaeus]

#904 was merged, closing here.

[OleksiiChernomaz]

while it's not directly related to the change, still sometimes confusing, because DriverException can contain "ConnectionException", e.g SQLSTATE[HY000] [1045] Access denied for user XYZ, but in the handler it's message replaced with following message An exception occured while establishing a connection to figure out your platform version. You can circumvent this by setting a 'server_version' configuration value...., which is at some point makes even more confusion as Previous exceptions not all the time logged properly.

IMO, maybe in general makes no sense to catch it? from trace will be clear when it has happened and from message will be clear why

[stof]

@OleksiiChernomaz but that message is still meaningful: in general, you don't want your connection to be established just to get the database platform (as that would require having access to the DB server during the time you warm up your metadata cache of the ORM, which might not be the case).
Specifying server_version is the right fix in that case.

If your credentials are indeed wrong (a user not having access to the DB, rather than just not having access during the cache warmup), then you will get an exception later when trying to connect to the DB server for a legitimate reason.

[OleksiiChernomaz]

while i agree that there is quite understandable logic in having message like "Occurred exception on attempt to figure out your platform version" on the other side sometimes it's confusing when you hit this error first time because IMO, it become understandable after you've already faced an issue and have seen this code. Thats why practically simpler when exception is not caught and bubbles up as not all the time previous exceptions are logged correctly, and based on the exception will be quite clear what has happened and from which piece of code it was triggered.

E.g i usually (probably as many other users as well) try to allow app to resolve version automatically to have less config options related to infra "hardcoded" on the app layer, thats why did not define on intend server_version and did not make it parametrised (because it will be an overkill). In my case error was because of the misconfigured DB server :)