cypht-org / cypht

Cypht: Lightweight Open Source webmail aggregator [PHP, JS]
http://cypht.org
GNU Lesser General Public License v2.1
949 stars 146 forks source link

Raise exceptions on error #1017

Open jonocodes opened 1 month ago

jonocodes commented 1 month ago

🗣 Suggestion

This conversation started here: https://github.com/cypht-org/cypht/issues/977#issuecomment-2098904257

Concerning PDO error modes php.watch/versions/8.0/PDO-default-errmode.

It appears php 7.x defaulted to make them silent, but 8.x is making them throw exceptions. This would have very different behaviors in the way cypht uses them since it is not explicitly setting a mode. So I recommend setting it explicitly.

That being said, I agree with the decision to have it default to throw exceptions. I came upon this when trying to figure out why some database operations were not working while I was developing. cypht seems to capture and hide the actual errors as you can see here: https://github.com/cypht-org/cypht/blob/2cf7a0ae88215e9ebc10935a0910290cec17fc5d/scripts/create_account.php#L43-L50

Unless there is another way to expose these errors (perhaps via a debug mode log) you cant see what the issue is, which is needed in order to fix it.

In my testing I was able to turn the above into a single line:

    $auth->create($user, $pass);

I explicitly ask for exceptions to happen when setting up the connection:

self::$dbh[$key]->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

That way the (error) logs are actionable.