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

Generic Exceptions raised in MySql::Connection #109

Open
brodyhoskins opened this issue Aug 21, 2023 · 6 comments · May be fixed by #110
Open

Generic Exceptions raised in MySql::Connection #109

brodyhoskins opened this issue Aug 21, 2023 · 6 comments · May be fixed by #110

Comments

@brodyhoskins
Copy link

Working with this shard there are specific exceptions I'd like to rescue from, but it looks like for the most part Mysql::Connection raises generic Exceptions.

I'd like to throw out the idea of using more specific exception types and see what the maintainer and/or community thinks. I'm also willing to open a PR for the change if anyone's interested.

@bcardiff
Copy link
Member

Adding specific exceptions sounds good. Feel free to send a PR or explain what use cases you have in mind.

@brodyhoskins
Copy link
Author

The context is:

I'm utilizing this shard within another small shard (run as an executable) that connects to the database every so often (long story) within Kubernetes. Due to the raised database connection error my shard crashes, the container crashes, then the entire pod crashes.

My goal is to handle specific exceptions so as to keep the loop happy and not bring down an entire Kubernetes pod. 😅

I'll submit a PR later on.

@bcardiff
Copy link
Member

If you use a connection pool instead of a direct connection you should be resilient to connection errors. I'm not sure how you are setting up/executing queries

@brodyhoskins
Copy link
Author

It's mostly related to credentials which have expired. Instead of crashing due to the exception it would be better for us to handle it specifically — to resolve it involves work outside of the container in question; it would eventually be killed off.

I just don't want to rescue every exception because if something more serious is happening it would be better to deal with a standard noisy crash than skipping it on by.

@bcardiff
Copy link
Member

If credentials are wrong I would expect you to get a DB::ConnectionRefused. Isn't that the case?

@brodyhoskins
Copy link
Author

Now that I think of it, I have seen DB::ConnectionRefused, but going back the exception encountered was:

Unhandled exception: Access denied for user '[REDACTED]'@'[REDACTED]' (using password: YES) (Exception)
  from app/lib/mysql/src/mysql/connection.cr:102:5 in 'handle_err_packet'
  from app/lib/mysql/src/mysql/connection.cr:116:7 in 'initialize'
  from app/lib/mysql/src/mysql/connection.cr:4:3 in 'new'
  from app/lib/mysql/src/mysql/driver.cr:3:5 in 'build_connection'
  from app/lib/db/src/db/database.cr:57:9 in '->'
  from app/lib/db/src/db/pool.cr:218:27 in 'build_resource'
  from app/lib/db/src/db/pool.cr:47:34 in 'initialize'
  from app/lib/db/src/db/pool.cr:40:5 in 'new:initial_pool_size:max_pool_size:max_idle_pool_size:checkout_timeout:retry_attempts:retry_delay'
  from app/lib/db/src/db/database.cr:56:15 in 'initialize'
  from app/lib/db/src/db/database.cr:49:5 in 'new'
  from app/lib/db/src/db.cr:155:5 in 'build_database'
  from app/lib/db/src/db.cr:151:5 in 'build_database'
  from app/lib/db/src/db.cr:124:5 in 'call'
  from app/src/leash.cr:54:3 in '__crystal_main'
  from usr/share/crystal/src/crystal/main.cr:115:5 in 'main_user_code'
  from usr/share/crystal/src/crystal/main.cr:101:7 in 'main'
  from usr/share/crystal/src/crystal/main.cr:127:3 in 'main'
  from /lib/ld-musl-x86_64.so.1 in '??'

@brodyhoskins brodyhoskins linked a pull request Aug 22, 2023 that will close this issue
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