Raise error if dbsqlsend fails

[andyundso]

Closes #464 Relates to #552

With this PR, tiny_tds will raise an error in case dbsqlsend will return FAIL instead of returning false. It is essentially the code from #469 / @wpolicarpo.

I spent an evening trying to build a test that would yield this error. I came up with a solution that worked on my machine, but only sometimes on CI.

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 also tried various suggestions to kill the session or just restart the database server, none of them worked for me to trigger the error. I personally think we can still get this code in even if it has no tests, as it can avoid getting into weird state with tiny_tds.

[aharpervc]

Idea, try adding sleep 1 in thread1 after the statement, eg

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

[andyundso]

I pushed the test now @aharpervc. As you can see, it sometimes works, sometimes not. So it really depends on a race-conditon between the two threads.

[aharpervc]

Yeah, unfortunate, I thought we could force an order of operations with the sleep. It seemed to be reliable for me locally with that technique. Is there a way to kill the client without threads?

[andyundso]

Hi @aharpervc

I experimented this evening again and I did not find any way to write a test that could simulate a FAIL from dbsqlsend. The primary case where this could happen is when the SQL server goes down between us checking if it is available with REQUIRE_OPEN_CLIENT and then running dbsqlsend, as illustrated below.

REQUIRE_OPEN_CLIENT(cwrap);
dbcmd(cwrap->client, StringValueCStr(sql));
if (dbsqlsend(cwrap->client) == FAIL) {
  rb_raise(cTinyTdsError, "failed dbsqlsend() function");
}

I assume the reason why the thread-based test sometimes work is because the userdata or the command buffer of FreeTDS is manipulated at the correct moment, and so the test fails. which, like we saw, is not reproducable really.

I now removed the commit with the test and suggest to merge this change as it is.