Improving `DBConnection.ConnectionError` message
akash-akya opened this issue · 7 comments
Currently, DBConnection.ConnectionError message only contains error description but does not contain the address information to which it is trying to connect. This makes debugging hard when the app is connecting to multiple MySQL databases.
Example error log (its getting logged in db_connection):
MyXQL.Connection (#PID<*.*.*>) failed to connect: ** (DBConnection.ConnectionError) connection refused
Can we include address information in the DBConnection.ConnectionError error message?
Mongodb lib does something similar.
Mongo.Protocol (#PID<*.*.*>) failed to connect: ** (Mongo.Error) 127.0.0.1:27017 tcp connect: connection refused - :econnrefused
Hi @wojtekmach, thanks for the quick update!
Can we change the message for all DBConnection.ConnectionError errors?
To have this I think we need to keep address in the state, is this okay?
I believe the errors you're pointing to would only happen after we've connected (e.g. socket got closed when doing a query, etc) so I don't think we need to annotate them with socket/host.
In fact, instead of %DBConnection.ConnectionError{} they probably could be %MyXQL.Error{mysql: nil}. Could you send a patch?
This error can also happen for ping right? Because of this in our logs we have many "db disconnected" error logs without context.
MyXQL.Connection (#PID<0.1205.0>) disconnected: ** (DBConnection.ConnectionError) socket closed
yup, exactly.
You mean returning %MyXQL.Error{mysql: nil, message: format_reason(reason)}? That will still produce generic error log without context right?
It will be something like
MyXQL.Connection (#PID<0.645.0>) disconnected: ** (MyXQL.Error) socket closed
Back to our use case, we have several MySQL databases connected at once (some of them will be idle for a some time). With the above log it is still hard to say which db is having network issue?
Please let me know if I'm missing something.
Yes, that was the idea.
We have the connection_id field on the MyXQL.Error struct, would including it in the message help?
I see. connection_id won't tell us anything though. Anyway since we have address information in the "connect" errors now. That should cover most of the basic cases.