Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise exceptions on error #1017

Closed
jonocodes opened this issue May 10, 2024 · 0 comments · Fixed by #1257
Closed

Raise exceptions on error #1017

jonocodes opened this issue May 10, 2024 · 0 comments · Fixed by #1257
Assignees

Comments

@jonocodes
Copy link
Contributor

🗣 Suggestion

This conversation started here: #977 (comment)

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:

if ($auth->create($user, $pass) === 2) {
die("User '" . $user . "' created\n\n");
}
else {
print_r(Hm_Debug::get());
print_r(Hm_Msgs::get());
die("An error occured when creating user '" . $user . "'\n\n");
}

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants