Add more user friendly message to Connection#initialize #55
Conversation
src/mysql/connection.cr
Outdated
| rescue Errno | ||
| raise DB::ConnectionRefused.new | ||
| rescue ex : Errno | ||
| raise DB::ConnectionRefused.new(ex.message) |
There was a problem hiding this comment.
How about setting the exception as the cause: DB::ConnectionRefused.new(cause: ex). This is what the PG driver does.
There was a problem hiding this comment.
While I agree that is a nicer way in theory, it'd make the error message equal to what it is now, which would defeat the point behind this PR. Using the sample code as given in the Crystal GitBook and running it on a machine that doesn't have MySQL running, the current message is " (DB::ConnectionRefused)", while my PR changes it to "Error connecting to 'localhost:3306': Connection refused (DB::ConnectionRefused)", which in my opinion is clearer than the current message.
There was a problem hiding this comment.
In the next version of crystal it will print out the cause of the exception, so it depends on if the next crystal-db release will be before or after 0.25.0
There was a problem hiding this comment.
Is DB::ConnectionRefused.new(cause: ex, message: ex.message) an acceptable variant then? This does print the nice error message on 0.24.2
There was a problem hiding this comment.
Yeah I guess that's fine for now. After 0.25.0 is released, we should probably refactor DB::ConnectionRefused to have a single constructor which takes the cause, and the exception message should always be "Connection to the database was refused".
There was a problem hiding this comment.
I've made the requested change. With respect to 0.25.0, I think it is better to give as much detail as possible, even returning the name of the database that is being connected to can be helpful to expose obvious configuration errors.
This adds the cause for the exception as a message to this exception, making it more clear why this failed.
|
To be honest, |
This adds the cause for the exception as a message to this exception,
making it more clear why this failed.
This saves a lot of debugging time when you have an issue with your database connection. As this is the first thing new users will do with this library, it is important to give users feedback on why this failed.