Avoid exception when db-connection is lost #717
Conversation
The tiny_tds lib might return false on execute when the call fails. This caused exceptions when active? is called after the connection was lost. fixes rails-sqlserver#707
| @@ -281,7 +281,8 @@ def sp_executesql_sql(sql, types, params, name) | |||
| def raw_connection_do(sql) | |||
| case @connection_options[:mode] | |||
| when :dblib | |||
| @connection.execute(sql).do | |||
| result = @connection.execute(sql) | |||
| result == false ? false : result.do | |||
There was a problem hiding this comment.
I know this is essentially the same thing, but
result && result.domight be more concise.
I did not review the functionality of the change, though.
I am just telling you an idiomatic ruby trick.
There was a problem hiding this comment.
No, it's not the same.
result && result.do
will return result if it is any falsy value.
When you look at the tiny_tds lib, you will see that it should either returns false or a result object which then can be called do on it.
In any other case - for example nil - I don't want it to return nil, but cause an exception, so it's easier to figure out what is going wrong.
| @@ -152,8 +152,7 @@ def disable_referential_integrity | |||
|
|
|||
| def active? | |||
| return false unless @connection | |||
| raw_connection_do 'SELECT 1' | |||
| true | |||
| raw_connection_do('SELECT 1') == 1 | |||
There was a problem hiding this comment.
IMO the previous one before the change is more robust.
Because the return value is independent of raw_connection_do implementation.
No matter how raw_connection_do changes in the future, as long as still raise expected exceptions.
This function will still work correctly.
There was a problem hiding this comment.
The problem with the previous version is that if raw_connection do does not return 1, but for example false, like it currently does when the connection is gone, no exception is raised which caused active? to return true, although the connection is not working.
With this change it will now only return true if the connection actually works, and false if an exception is raised, or anything else causes the connection to not return proper values any more.
There was a problem hiding this comment.
Additionally I'd like to point out that I did not get rid of the rescue block, so the previous behaviour of returning false if an connection_errors is raised is still the same.
|
Could you elaborate more why or how I am quite interested in this case now. This can potential be a bug of the underlying connection layer. |
Its source is in the c-implementaion of the connection adapter: https://github.com/rails-sqlserver/tiny_tds/blob/master/ext/tiny_tds/client.c#L256 The simplest way I found to reproduce this locally is by using a docker-container for the database, open a Rails console and make any db-request, so a connection is opened. Then restart the database container without closing the Rails console and after the database docker container is up again, call the |
|
I opened an issue on TinyTds. (rails-sqlserver/tiny_tds#451) In the meantime, I found out that @metaskills To summarize the options now are:
B) and C) are not mutually exclusive, both can be done. |
|
From my experience |
|
Hi. I'm jumping into this issue now. I would prefer to have this fixed at My only questions are
@Svelix do you mind rebasing your branch so we can check for any failing tests? |
TinyTDS returns false instead of raising an exception if connection fails while executing a query. Getting around this by raising an exception ourselves while this PR rails-sqlserver/tiny_tds#469 is not released. The solution came from #717 and #818, I am just adjusting the error message to make it the same as that TinyTDS PR.
TinyTDS returns false instead of raising an exception if connection fails while executing a query. Getting around this by raising an exception ourselves while this PR rails-sqlserver/tiny_tds#469 is not released. The solution came from #717 and #818, I am just adjusting the error message to make it the same as that TinyTDS PR.
|
Closing this via #818, #825 and rails-sqlserver/tiny_tds/pull/469. |
The tiny_tds lib might return false on execute when the call fails.
This caused exceptions when active? is called after the connection
was lost.
fixes #707