Result from a dead connection is false. Should it throw an exception?

[jrgns]

Before submitting an issue please check these first!

• On Windows? If so, do you need devkit for your ruby install?
• Using Ubuntu? If so, you may have forgotten to install FreeTDS first.
• Are you using FreeTDS 0.95.80 or later? Check $ tsql -C to find out.
• If not, please update then uninstall the TinyTDS gem and re-install it.
• Have you made sure to enable SQL Server authentication?
• Doing work with threads and the raw client? Use the ConnectionPool gem?

If none of these help. Please fill out the following:

Environment

Operating System

lsb_release -a; uname -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 18.04.4 LTS
Release:	18.04
Codename:	bionic
Linux app04 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

TinyTDS Version and Information

tsql -C
[TinyTds][v1.3.0][tsql]: /usr/local/bin/tsql
Compile-time settings (established with the "configure" script)
                            Version: freetds v1.1.24
             freetds.conf directory: /usr/local/etc
     MS db-lib source compatibility: no
        Sybase binary compatibility: no
                      Thread safety: yes
                      iconv library: yes
                        TDS version: auto
                              iODBC: no
                           unixodbc: yes
              SSPI "trusted" logins: no
                           Kerberos: no
                            OpenSSL: no
                             GnuTLS: no
                               MARS: yes

Description

Previously posted this on the sequel gem. It looks like there's an expectation for the TinyTDS client to raise an exception when the connection is dead, not return false.

The following shows that the client is returning false:

require 'sequel'
require 'logger'
DB = Sequel.connect(ENV['DATABASE_URL'])
DB.extension(:connection_validator)
DB.loggers << Logger.new($stdout)
DB[:table].first # Successful
# Externally kill the connection to the DB
DB[:table].first # Reports the error below

/home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/adapters/tinytds.rb:73: warning: TinyTds: dbsqlsend() returned FAIL.
Traceback (most recent call last):
       16: from /home/aex/.rvm/gems/ruby-2.7.0/bin/irb:23:in `<main>'
       15: from /home/aex/.rvm/gems/ruby-2.7.0/bin/irb:23:in `load'
       14: from /home/aex/.rvm/rubies/ruby-2.7.0/lib/ruby/gems/2.7.0/gems/irb-1.2.1/exe/irb:11:in `<top (required)>'
       13: from (irb):9
       12: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/dataset/actions.rb:208:in `first'
       11: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/dataset/actions.rb:693:in `single_record'
       10: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/dataset/actions.rb:705:in `single_record!'
        9: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/dataset/actions.rb:950:in `with_sql_first'
        8: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/dataset/actions.rb:942:in `with_sql_each'
        7: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/adapters/tinytds.rb:214:in `fetch_rows'
        6: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/dataset/actions.rb:1089:in `execute'
        5: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/adapters/tinytds.rb:46:in `execute'
        4: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/database/connecting.rb:270:in `synchronize'
        3: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/connection_pool/threaded.rb:92:in `hold'
        2: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/adapters/tinytds.rb:77:in `block in execute'
        1: from /home/aex/fno-interface-stage/vendor/bundle/ruby/2.7.0/gems/sequel-5.32.0/lib/sequel/adapters/tinytds.rb:217:in `block in fetch_rows'
NoMethodError (undefined method `fields' for false:FalseClass)

[aharpervc]

I've seen this before as well, and I don't like it... I agree TinyTDS should do a better job with this in my opinion.

[andyundso]

I tried to write a test this evening to reproduce this error (basically that #execute returns false). I tried various Toxiproxy configurations, does not work. I also tried to kill a session, this brings up Cannot continue the execution because the session is in the kill state.

What works consistently on my machine (and about 90% of the time on CI) is the following:

it 'raises error when query cannot be sent' do
  client = new_connection
  assert_client_works(client)

  thread1 = Thread.new do
    client.execute("SELECT 1 as [one]").do
  end

  thread2 = Thread.new do
    assert_equal false, client.execute("SELECT 1 as [one]")
  end

  thread1.join
  thread2.join
end

I assume, when the result are read in thread1 and Ruby decides to switch to thread2, our FreeTDS wrapper is in an invalid state, which can trigger the issue. Likely, because they both try to access the DBPROCESS object, which should not be allowed.

I would suggest: We implement the exception as suggest in #469, but without any tests, as long as we do not know how to reliably trigger it. Still, I think the exception would be an improvement, since it prevents subsequents errors for users.