From 6368315c89e5038f32234dccea8cba78fa4d2863 Mon Sep 17 00:00:00 2001
From: Wanderson Policarpo <wpolicarpo@gmail.com>
Date: Wed, 13 May 2020 22:23:35 +0100
Subject: [PATCH 1/5] Raise error if client is unable to execute statement

---
 ext/tiny_tds/client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c
index f7898a74..12a29e3d 100644
--- a/ext/tiny_tds/client.c
+++ b/ext/tiny_tds/client.c
@@ -252,8 +252,8 @@ static VALUE rb_tinytds_execute(VALUE self, VALUE sql) {
   REQUIRE_OPEN_CLIENT(cwrap);
   dbcmd(cwrap->client, StringValueCStr(sql));
   if (dbsqlsend(cwrap->client) == FAIL) {
-    rb_warn("TinyTds: dbsqlsend() returned FAIL.\n");
-    return Qfalse;
+    rb_raise(cTinyTdsError, "failed to execute statement");
+    return self;
   }
   cwrap->userdata->dbsql_sent = 1;
   result = rb_tinytds_new_result_obj(cwrap);

From 0cbc0a32bb5b4189b4da55a962a38f43d03c0396 Mon Sep 17 00:00:00 2001
From: bvogelzang <ben.vogelzang@veracross.com>
Date: Fri, 2 Oct 2020 14:22:28 -0500
Subject: [PATCH 2/5] Better handling of network related timeouts

With a working network connection:
* command batch is called on non gvl thread
* tinytds_err_handler is called with timeout error and returns INT_TIMEOUT
* dbcancel is called and command batch returns
* nogvl_cleanup is called and timeout error is raised

This is all great. The timeout is hit, the db connection lives, and a error is thrown.

With a network failure:
* command batch is called on non gvl thread
* tinytds_err_handler is called with timeout error and returns INT_TIMEOUT
* dbcancel is called and does not succeed. command batch never returns
* nogvl_cleanup is not called

This is not great. dbcancel does not succeed because of the network failure. the command batch does not return until the underlying network timeout on the os is hit. TinyTds doesn't throw an error in the expected timeout window.

To fix, we set a flag when a timeout is encountered. We use dbsetinterrupt to check this flag periodically while waiting on a read from the server. Once the flag is set the interrupt with send INT_CANCEL causing the pending command batch to return early. This means nogvl_cleanup will be called and raise the timeout error.

This shouldn't have any affect in "normal" timeout conditions due to the fact that dbcancel will actually succeed and cause the normal flow before the interrupt can be called/handled. This is good because in these situtations we still want the dbproc to remain in working condition.

Also, update severity assertion to support more freetds versions. Severity level changed with this commit in freetds https://github.com/FreeTDS/freetds/commit/66423608f405d5714b047a67652d8a6015094f29. Since the severity check isn't essential we allow for either severity in order to support a wider range of freetds versions.
---
 .travis.yml           |  2 +-
 CHANGELOG.md          |  3 ++-
 README.md             | 17 +++++++++----
 docker-compose.yml    | 22 +++++++++++++++++
 ext/tiny_tds/client.c | 36 ++++++++++++++++++++++++++-
 ext/tiny_tds/result.c |  1 +
 test/client_test.rb   | 57 ++++++++++++++++++++++++++++---------------
 test/test_helper.rb   | 22 ++++++++++++++++-
 tiny_tds.gemspec      |  1 +
 9 files changed, 133 insertions(+), 28 deletions(-)
 create mode 100644 docker-compose.yml

diff --git a/.travis.yml b/.travis.yml
index dd9430b4..1ea36f00 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -14,9 +14,9 @@ rvm:
   - 2.7.0
 before_install:
   - docker info
+  - docker-compose up -d
   - sudo ./test/bin/install-openssl.sh
   - sudo ./test/bin/install-freetds.sh
-  - sudo ./test/bin/setup.sh
 install:
   - gem install bundler
   - bundle --version
diff --git a/CHANGELOG.md b/CHANGELOG.md
index db1ce76f..94d21f31 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## (unreleased)
 
+* Improve handling of network related timeouts
+
 ## 2.1.3
 
 * Removed old/unused appveyor config
@@ -262,4 +264,3 @@ Use both dbsetversion() vs. dbsetlversion. Partially reverts #62.
 
 
 ## 0.1.0 Initial release!
-
diff --git a/README.md b/README.md
index 09135e93..de7ed6d2 100644
--- a/README.md
+++ b/README.md
@@ -110,7 +110,7 @@ Creating a new client takes a hash of options. For valid iconv encoding options,
 * :appname - Short string seen in SQL Servers process/activity window.
 * :tds_version - TDS version. Defaults to "7.3".
 * :login_timeout - Seconds to wait for login. Default to 60 seconds.
-* :timeout - Seconds to wait for a response to a SQL command. Default 5 seconds. Prior to 1.0rc5, FreeTDS was unable to set the timeout on a per-client basis, permitting only a global timeout value. This means that if you're using an older version, the timeout values for all clients will be overwritten each time you instantiate a new `TinyTds::Client` object. If you are using 1.0rc5 or later, all clients will have an independent timeout setting as you'd expect.
+* :timeout - Seconds to wait for a response to a SQL command. Default 5 seconds. Prior to 1.0rc5, FreeTDS was unable to set the timeout on a per-client basis, permitting only a global timeout value. This means that if you're using an older version, the timeout values for all clients will be overwritten each time you instantiate a new `TinyTds::Client` object. If you are using 1.0rc5 or later, all clients will have an independent timeout setting as you'd expect. Timeouts caused by network failure will raise a timeout error 1 second after the configured timeout limit is hit (see [#481](https://github.com/rails-sqlserver/tiny_tds/pull/481) for details).
 * :encoding - Any valid iconv value like CP1251 or ISO-8859-1. Default UTF-8.
 * :azure - Pass true to signal that you are connecting to azure.
 * :contained - Pass true to signal that you are connecting with a contained database user.
@@ -322,6 +322,10 @@ By default row caching is turned on because the SQL Server adapter for ActiveRec
 TinyTDS takes an opinionated stance on how we handle encoding errors. First, we treat errors differently on reads vs. writes. Our opinion is that if you are reading bad data due to your client's encoding option, you would rather just find `?` marks in your strings vs being blocked with exceptions. This is how things wold work via ODBC or SMS. On the other hand, writes will raise an exception. In this case we raise the SYBEICONVO/2402 error message which has a description of `Error converting characters into server's character set. Some character(s) could not be converted.`. Even though the severity of this message is only a `4` and TinyTDS will automatically strip/ignore unknown characters, we feel you should know that you are inserting bad encodings. In this way, a transaction can be rolled back, etc. Remember, any database write that has bad characters due to the client encoding will still be written to the database, but it is up to you rollback said write if needed. Most ORMs like ActiveRecord handle this scenario just fine.
 
 
+## Timeout Error Handling
+
+TinyTDS will raise a `TinyTDS::Error` when a timeout is reached based on the options supplied to the client. Depending on the reason for the timeout, the connection could be dead or alive. When db processing is the cause for the timeout, the connection should still be usable after the error is raised. When network failure is the cause of the timeout, the connection will be dead. If you attempt to execute another command batch on a dead connection you will see a `DBPROCESS is dead or not enabled` error. Therefore, it is recommended to check for a `dead?` connection before trying to execute another command batch.
+
 ## Binstubs
 
 The TinyTDS gem uses binstub wrappers which mirror compiled [FreeTDS Utilities](https://www.freetds.org/userguide/usefreetds.html) binaries. These native executables are usually installed at the system level when installing FreeTDS. However, when using MiniPortile to install TinyTDS as we do with Windows binaries, these binstubs will find and prefer local gem `exe` directory executables. These are the following binstubs we wrap.
@@ -419,17 +423,20 @@ First, clone the repo using the command line or your Git GUI of choice.
 $ git clone git@github.com:rails-sqlserver/tiny_tds.git
 ```
 
-After that, the quickest way to get setup for development is to use [Docker](https://www.docker.com/). Assuming you have [downloaded docker](https://www.docker.com/products/docker) for your platform and you have , you can run our test setup script.
+After that, the quickest way to get setup for development is to use [Docker](https://www.docker.com/). Assuming you have [downloaded docker](https://www.docker.com/products/docker) for your platform, you can use [docker-compose](https://docs.docker.com/compose/install/) to run the necessary containers for testing.
 
 ```shell
-$ ./test/bin/setup.sh
+$ docker-compose up -d
 ```
 
-This will download our SQL Server for Linux Docker image based from [microsoft/mssql-server-linux/](https://hub.docker.com/r/microsoft/mssql-server-linux/). Our image already has the `[tinytdstest]` DB and `tinytds` users created. Basically, it does the following.
+This will download our SQL Server for Linux Docker image based from [microsoft/mssql-server-linux/](https://hub.docker.com/r/microsoft/mssql-server-linux/). Our image already has the `[tinytdstest]` DB and `tinytds` users created. This will also download a [toxiproxy](https://github.com/shopify/toxiproxy) Docker image which we can use to simulate network failures for tests. Basically, it does the following.
 
 ```shell
+$ docker network create main-network
 $ docker pull metaskills/mssql-server-linux-tinytds
-$ docker run -p 1433:1433 -d metaskills/mssql-server-linux-tinytds
+$ docker run -p 1433:1433 -d --name sqlserver --network main-network metaskills/mssql-server-linux-tinytds
+$ docker pull shopify/toxiproxy
+$ docker run -p 8474:8474 -p 1234:1234 -d --name toxiproxy --network main-network shopify/toxiproxy
 ```
 
 If you are using your own database. Make sure to run these SQL commands as SA to get the test database and user installed.
diff --git a/docker-compose.yml b/docker-compose.yml
new file mode 100644
index 00000000..e42d4c3a
--- /dev/null
+++ b/docker-compose.yml
@@ -0,0 +1,22 @@
+version: '3'
+
+networks:
+  main-network:
+
+services:
+  mssql:
+    image: metaskills/mssql-server-linux-tinytds:2017-GA
+    container_name: sqlserver
+    ports:
+      - "1433:1433"
+    networks:
+      - main-network
+
+  toxiproxy:
+    image: shopify/toxiproxy
+    container_name: toxiproxy
+    ports:
+      - "8474:8474"
+      - "1234:1234"
+    networks:
+      - main-network
diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c
index 12a29e3d..846ecbbc 100644
--- a/ext/tiny_tds/client.c
+++ b/ext/tiny_tds/client.c
@@ -86,7 +86,13 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c
       but we don't ever want to automatically retry. Instead have the app
       decide what to do.
       */
-      return_value = INT_TIMEOUT;
+      if (userdata->timing_out) {
+        return INT_CANCEL;
+      }
+      else {
+        userdata->timing_out = 1;
+        return_value = INT_TIMEOUT;
+      }
       cancel = 1;
       break;
 
@@ -165,6 +171,33 @@ int tinytds_msg_handler(DBPROCESS *dbproc, DBINT msgno, int msgstate, int severi
   return 0;
 }
 
+/*
+Used by dbsetinterrupt -
+This gets called periodically while waiting on a read from the server
+Right now, we only care about cases where a read from the server is
+taking longer than the specified timeout and dbcancel is not working.
+In these cases we decide that we actually want to handle the interrupt
+*/
+static int check_interrupt(void *ptr) {
+  GET_CLIENT_USERDATA((DBPROCESS *)ptr);
+  return userdata->timing_out;
+}
+
+/*
+Used by dbsetinterrupt -
+This gets called if check_interrupt returns TRUE.
+Right now, this is only used in cases where a read from the server is
+taking longer than the specified timeout and dbcancel is not working.
+Return INT_CANCEL to abort the current command batch.
+*/
+static int handle_interrupt(void *ptr) {
+  GET_CLIENT_USERDATA((DBPROCESS *)ptr);
+  if (userdata->timing_out) {
+    return INT_CANCEL;
+  }
+  return INT_CONTINUE;
+}
+
 static void rb_tinytds_client_reset_userdata(tinytds_client_userdata *userdata) {
   userdata->timing_out = 0;
   userdata->dbsql_sent = 0;
@@ -381,6 +414,7 @@ static VALUE rb_tinytds_connect(VALUE self, VALUE opts) {
       }
     }
     dbsetuserdata(cwrap->client, (BYTE*)cwrap->userdata);
+    dbsetinterrupt(cwrap->client, check_interrupt, handle_interrupt);
     cwrap->userdata->closed = 0;
     if (!NIL_P(database) && (azure != Qtrue)) {
       dbuse(cwrap->client, StringValueCStr(database));
diff --git a/ext/tiny_tds/result.c b/ext/tiny_tds/result.c
index 7535e6cb..3b7ad7cb 100644
--- a/ext/tiny_tds/result.c
+++ b/ext/tiny_tds/result.c
@@ -91,6 +91,7 @@ static void nogvl_setup(DBPROCESS *client) {
 static void nogvl_cleanup(DBPROCESS *client) {
   GET_CLIENT_USERDATA(client);
   userdata->nonblocking = 0;
+  userdata->timing_out = 0;
   /*
   Now that the blocking operation is done, we can finally throw any
   exceptions based on errors from SQL Server.
diff --git a/test/client_test.rb b/test/client_test.rb
index ac8feaeb..15a0421e 100644
--- a/test/client_test.rb
+++ b/test/client_test.rb
@@ -68,6 +68,9 @@ class ClientTest < TinyTds::TestCase
   end
 
   describe 'With in-valid options' do
+    before(:all) do
+      init_toxiproxy
+    end
 
     it 'raises an argument error when no :host given and :dataserver is blank' do
       assert_raises(ArgumentError) { new_connection :dataserver => nil, :host => nil }
@@ -129,30 +132,46 @@ class ClientTest < TinyTds::TestCase
       end
     end
 
-    it 'must run this test to prove we account for dropped connections' do
-      skip
+    it 'raises TinyTds exception with tcp socket network failure' do
+      skip if ENV['CI'] && ENV['APPVEYOR_BUILD_FOLDER'] # only CI using docker
       begin
-        client = new_connection :login_timeout => 2, :timeout => 2
+        client = new_connection timeout: 2, port: 1234
         assert_client_works(client)
-        STDOUT.puts "Disconnect network!"
-        sleep 10
-        STDOUT.puts "This should not get stuck past 6 seconds!"
-        action = lambda { client.execute('SELECT 1 as [one]').each }
-        assert_raise_tinytds_error(action) do |e|
-          assert_equal 20003, e.db_error_number
-          assert_equal 6, e.severity
-          assert_match %r{timed out}i, e.message, 'ignore if non-english test run'
+        action = lambda { client.execute("waitfor delay '00:00:05'").do }
+
+        # Use toxiproxy to close the TCP socket after 1 second.
+        # We want TinyTds to execute the statement, hit the timeout configured above, and then not be able to use the network to cancel
+        # the network connection needs to close after the sql batch is sent and before the timeout above is hit
+        Toxiproxy[:sqlserver_test].toxic(:slow_close, delay: 1000).apply do
+          assert_raise_tinytds_error(action) do |e|
+            assert_equal 20003, e.db_error_number
+            assert_equal 6, e.severity
+            assert_match %r{timed out}i, e.message, 'ignore if non-english test run'
+          end
         end
       ensure
-        STDOUT.puts "Reconnect network!"
-        sleep 10
-        action = lambda { client.execute('SELECT 1 as [one]').each }
-        assert_raise_tinytds_error(action) do |e|
-          assert_equal 20047, e.db_error_number
-          assert_equal 1, e.severity
-          assert_match %r{dead or not enabled}i, e.message, 'ignore if non-english test run'
+        assert_new_connections_work
+      end
+    end
+
+    it 'raises TinyTds exception with dead connection network failure' do
+      skip if ENV['CI'] && ENV['APPVEYOR_BUILD_FOLDER'] # only CI using docker
+      begin
+        client = new_connection timeout: 2, port: 1234
+        assert_client_works(client)
+        action = lambda { client.execute("waitfor delay '00:00:05'").do }
+
+        # Use toxiproxy to close the network connection after 1 second.
+        # We want TinyTds to execute the statement, hit the timeout configured above, and then not be able to use the network to cancel
+        # the network connection needs to close after the sql batch is sent and before the timeout above is hit
+        Toxiproxy[:sqlserver_test].toxic(:timeout, timeout: 1000).apply do
+          assert_raise_tinytds_error(action) do |e|
+            assert_equal 20047, e.db_error_number
+            assert_includes [1,9], e.severity
+            assert_match %r{dead or not enabled}i, e.message, 'ignore if non-english test run'
+          end
         end
-        close_client(client)
+      ensure
         assert_new_connections_work
       end
     end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index fff8fd77..fc364b42 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -2,6 +2,7 @@
 require 'bundler' ; Bundler.require :development, :test
 require 'tiny_tds'
 require 'minitest/autorun'
+require 'toxiproxy'
 
 TINYTDS_SCHEMAS = ['sqlserver_2000', 'sqlserver_2005', 'sqlserver_2008', 'sqlserver_2014', 'sqlserver_azure', 'sybase_ase'].freeze
 
@@ -212,6 +213,25 @@ def rollback_transaction(client)
       client.execute("ROLLBACK TRANSACTION").do
     end
 
+    def init_toxiproxy
+      return if ENV['APPVEYOR_BUILD_FOLDER'] # only for CI using docker
+
+      # In order for toxiproxy to work for local docker instances of mssql, the containers must be on the same network
+      # and the host used below must match the mssql container name so toxiproxy knows where to proxy to.
+      # localhost from the perspective of toxiproxy's container is its own container an *not* the mssql container it needs to proxy to.
+      # docker-compose.yml handles this automatically for us. In instances where someone is using their own local mssql container they'll
+      # need to set up the networks manually and set TINYTDS_UNIT_HOST to their mssql container name
+      # For anything other than localhost just use the environment config
+      env_host = ENV['TINYTDS_UNIT_HOST_TEST'] || ENV['TINYTDS_UNIT_HOST'] || 'localhost'
+      host = ['localhost', '127.0.0.1', '0.0.0.0'].include?(env_host) ? 'sqlserver' : env_host
+      port = ENV['TINYTDS_UNIT_PORT'] || 1433
+      Toxiproxy.populate([
+        {
+          name: "sqlserver_test",
+          listen: "0.0.0.0:1234",
+          upstream: "#{host}:#{port}"
+        }
+      ])
+    end
   end
 end
-
diff --git a/tiny_tds.gemspec b/tiny_tds.gemspec
index e67513be..9a881a3a 100644
--- a/tiny_tds.gemspec
+++ b/tiny_tds.gemspec
@@ -26,4 +26,5 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'rake-compiler-dock', '~> 1.0'
   s.add_development_dependency 'minitest', '~> 5.6'
   s.add_development_dependency 'connection_pool', '~> 2.2'
+  s.add_development_dependency 'toxiproxy', '~> 2.0.0'
 end

From 2d198a36a6c03e7adfe041ab33e8aacdd353c702 Mon Sep 17 00:00:00 2001
From: bvogelzang <ben.vogelzang@veracross.com>
Date: Wed, 22 Jul 2020 15:35:24 -0500
Subject: [PATCH 3/5] Properly report stored procedure errors when using info
 messages - capturing errors while in a non-blocking state was originally
 structured to capture a single error. This was intentional in order to avoid
 capturing more generic info messages that FreeTDS might send before the
 Global VM Lock was obtained. In most circumstances this is what we want.
 However, now that we capture info messages it is possible that a info message
 will be stored and the actual runtime error will be discarded as
 non-important. The result is that while a runtime error is reported in the
 database, a TinyTds error is never thrown and only the info message is
 handled. A subset of this problem is that only one info message can be
 captured while in non-blocking mode which prevents stored procedures from
 reporting multiple info messages to TinyTds. - to fix this issue, the
 reported messages are stored within a dynamic array of tinytds_errordata
 structs, then processed normally once the GVL is obtained. - given the fact
 that we don't know the number of messages that will be sent, we dynamically
 manage and re-allocate memory for the nonblocking_errors array as needed. We
 can't use the ruby C API because it is not safe to call while in a
 non-blocking state as shown by #133.

---
 CHANGELOG.md          |  1 +
 ext/tiny_tds/client.c | 99 ++++++++++++++++++++++++-------------------
 ext/tiny_tds/client.h |  8 ++--
 ext/tiny_tds/result.c | 31 +++++++++-----
 test/result_test.rb   | 43 ++++++++++++++++++-
 test/test_helper.rb   | 38 ++++++++++++++++-
 6 files changed, 160 insertions(+), 60 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94d21f31..ed76d23b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## (unreleased)
 
 * Improve handling of network related timeouts
+* Fix error reporting when preceeded by info message
 
 ## 2.1.3
 
diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c
index 846ecbbc..0756cd03 100644
--- a/ext/tiny_tds/client.c
+++ b/ext/tiny_tds/client.c
@@ -24,25 +24,25 @@ VALUE opt_escape_regex, opt_escape_dblquote;
 
 // Lib Backend (Helpers)
 
-VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int is_message, int cancel, const char *error, const char *source, int severity, int dberr, int oserr) {
+VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, tinytds_errordata error) {
   VALUE e;
   GET_CLIENT_USERDATA(dbproc);
-  if (cancel && !dbdead(dbproc) && userdata && !userdata->closed) {
+  if (error.cancel && !dbdead(dbproc) && userdata && !userdata->closed) {
     userdata->dbsqlok_sent = 1;
     dbsqlok(dbproc);
     userdata->dbcancel_sent = 1;
     dbcancel(dbproc);
   }
-  e = rb_exc_new2(cTinyTdsError, error);
-  rb_funcall(e, intern_source_eql, 1, rb_str_new2(source));
-  if (severity)
-    rb_funcall(e, intern_severity_eql, 1, INT2FIX(severity));
-  if (dberr)
-    rb_funcall(e, intern_db_error_number_eql, 1, INT2FIX(dberr));
-  if (oserr)
-    rb_funcall(e, intern_os_error_number_eql, 1, INT2FIX(oserr));
-
-  if (severity <= 10 && is_message) {
+  e = rb_exc_new2(cTinyTdsError, error.error);
+  rb_funcall(e, intern_source_eql, 1, rb_str_new2(error.source));
+  if (error.severity)
+    rb_funcall(e, intern_severity_eql, 1, INT2FIX(error.severity));
+  if (error.dberr)
+    rb_funcall(e, intern_db_error_number_eql, 1, INT2FIX(error.dberr));
+  if (error.oserr)
+    rb_funcall(e, intern_os_error_number_eql, 1, INT2FIX(error.oserr));
+
+  if (error.severity <= 10 && error.is_message) {
     VALUE message_handler = userdata && userdata->message_handler ? userdata->message_handler : Qnil;
     if (message_handler && message_handler != Qnil && rb_respond_to(message_handler, intern_call) != 0) {
       rb_funcall(message_handler, intern_call, 1, e);
@@ -57,6 +57,16 @@ VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int is_message, int cancel, cons
 
 
 // Lib Backend (Memory Management & Handlers)
+static void push_userdata_error(tinytds_client_userdata *userdata, tinytds_errordata error) {
+  // reallocate memory for the array as needed
+  if (userdata->nonblocking_errors_size == userdata->nonblocking_errors_length) {
+    userdata->nonblocking_errors_size *= 2;
+    userdata->nonblocking_errors = realloc(userdata->nonblocking_errors, userdata->nonblocking_errors_size * sizeof(tinytds_errordata));
+  }
+
+  userdata->nonblocking_errors[userdata->nonblocking_errors_length] = error;
+  userdata->nonblocking_errors_length++;
+}
 
 int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, char *dberrstr, char *oserrstr) {
   static const char *source = "error";
@@ -105,6 +115,16 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c
       break;
   }
 
+  tinytds_errordata error_data = {
+    .is_message = 0,
+    .cancel = cancel,
+    .severity = severity,
+    .dberr = dberr,
+    .oserr = oserr
+  };
+  strncpy(error_data.error, dberrstr, ERROR_MSG_SIZE);
+  strncpy(error_data.source, source, ERROR_MSG_SIZE);
+
   /*
   When in non-blocking mode we need to store the exception data to throw it
   once the blocking call returns, otherwise we will segfault ruby since part
@@ -116,27 +136,9 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c
       dbcancel(dbproc);
       userdata->dbcancel_sent = 1;
     }
-
-    /*
-    If we've already captured an error message, don't overwrite it. This is
-    here because FreeTDS sends a generic "General SQL Server error" message
-    that will overwrite the real message. This is not normally a problem
-    because a ruby exception is normally thrown and we bail before the
-    generic message can be sent.
-    */
-    if (!userdata->nonblocking_error.is_set) {
-      userdata->nonblocking_error.is_message = 0;
-      userdata->nonblocking_error.cancel = cancel;
-      strncpy(userdata->nonblocking_error.error, dberrstr, ERROR_MSG_SIZE);
-      strncpy(userdata->nonblocking_error.source, source, ERROR_MSG_SIZE);
-      userdata->nonblocking_error.severity = severity;
-      userdata->nonblocking_error.dberr = dberr;
-      userdata->nonblocking_error.oserr = oserr;
-      userdata->nonblocking_error.is_set = 1;
-    }
-
+    push_userdata_error(userdata, error_data);
   } else {
-    rb_tinytds_raise_error(dbproc, 0, cancel, dberrstr, source, severity, dberr, oserr);
+    rb_tinytds_raise_error(dbproc, error_data);
   }
 
   return return_value;
@@ -148,25 +150,31 @@ int tinytds_msg_handler(DBPROCESS *dbproc, DBINT msgno, int msgstate, int severi
 
   int is_message_an_error = severity > 10 ? 1 : 0;
 
+  tinytds_errordata error_data = {
+    .is_message = !is_message_an_error,
+    .cancel = is_message_an_error,
+    .severity = severity,
+    .dberr = msgno,
+    .oserr = msgstate
+  };
+  strncpy(error_data.error, msgtext, ERROR_MSG_SIZE);
+  strncpy(error_data.source, source, ERROR_MSG_SIZE);
+
   // See tinytds_err_handler() for info about why we do this
   if (userdata && userdata->nonblocking) {
-    if (!userdata->nonblocking_error.is_set) {
-      userdata->nonblocking_error.is_message = !is_message_an_error;
-      userdata->nonblocking_error.cancel = is_message_an_error;
-      strncpy(userdata->nonblocking_error.error, msgtext, ERROR_MSG_SIZE);
-      strncpy(userdata->nonblocking_error.source, source, ERROR_MSG_SIZE);
-      userdata->nonblocking_error.severity = severity;
-      userdata->nonblocking_error.dberr = msgno;
-      userdata->nonblocking_error.oserr = msgstate;
-      userdata->nonblocking_error.is_set = 1;
-    }
+    /*
+    In the case of non-blocking command batch execution we can receive multiple messages
+    (including errors). We keep track of those here so they can be processed once the
+    non-blocking call returns.
+    */
+    push_userdata_error(userdata, error_data);
 
     if (is_message_an_error && !dbdead(dbproc) && !userdata->closed) {
       dbcancel(dbproc);
       userdata->dbcancel_sent = 1;
     }
   } else {
-    rb_tinytds_raise_error(dbproc, !is_message_an_error, is_message_an_error, msgtext, source, severity, msgno, msgstate);
+    rb_tinytds_raise_error(dbproc, error_data);
   }
   return 0;
 }
@@ -204,7 +212,10 @@ static void rb_tinytds_client_reset_userdata(tinytds_client_userdata *userdata)
   userdata->dbsqlok_sent = 0;
   userdata->dbcancel_sent = 0;
   userdata->nonblocking = 0;
-  userdata->nonblocking_error.is_set = 0;
+  // the following is mainly done for consistency since the values are reset accordingly in nogvl_setup/cleanup.
+  // the nonblocking_errors array does not need to be freed here. That is done as part of nogvl_cleanup.
+  userdata->nonblocking_errors_length = 0;
+  userdata->nonblocking_errors_size = 0;
 }
 
 static void rb_tinytds_client_mark(void *ptr) {
diff --git a/ext/tiny_tds/client.h b/ext/tiny_tds/client.h
index 5e5c811f..dcd6087c 100644
--- a/ext/tiny_tds/client.h
+++ b/ext/tiny_tds/client.h
@@ -5,9 +5,9 @@
 void init_tinytds_client();
 
 #define ERROR_MSG_SIZE 1024
+#define ERRORS_STACK_INIT_SIZE 2
 
 typedef struct {
-  short int is_set;
   int is_message;
   int cancel;
   char error[ERROR_MSG_SIZE];
@@ -25,7 +25,9 @@ typedef struct {
   RETCODE dbsqlok_retcode;
   short int dbcancel_sent;
   short int nonblocking;
-  tinytds_errordata nonblocking_error;
+  short int nonblocking_errors_length;
+  short int nonblocking_errors_size;
+  tinytds_errordata *nonblocking_errors;
   VALUE message_handler;
 } tinytds_client_userdata;
 
@@ -40,7 +42,7 @@ typedef struct {
   rb_encoding *encoding;
 } tinytds_client_wrapper;
 
-VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int is_message, int cancel, const char *error, const char *source, int severity, int dberr, int oserr);
+VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, tinytds_errordata error);
 
 // Lib Macros
 
diff --git a/ext/tiny_tds/result.c b/ext/tiny_tds/result.c
index 3b7ad7cb..8541c6b4 100644
--- a/ext/tiny_tds/result.c
+++ b/ext/tiny_tds/result.c
@@ -86,6 +86,9 @@ static void dbcancel_ubf(DBPROCESS *client) {
 static void nogvl_setup(DBPROCESS *client) {
   GET_CLIENT_USERDATA(client);
   userdata->nonblocking = 1;
+  userdata->nonblocking_errors_length = 0;
+  userdata->nonblocking_errors = malloc(ERRORS_STACK_INIT_SIZE * sizeof(tinytds_errordata));
+  userdata->nonblocking_errors_size = ERRORS_STACK_INIT_SIZE;
 }
 
 static void nogvl_cleanup(DBPROCESS *client) {
@@ -96,17 +99,25 @@ static void nogvl_cleanup(DBPROCESS *client) {
   Now that the blocking operation is done, we can finally throw any
   exceptions based on errors from SQL Server.
   */
-  if (userdata->nonblocking_error.is_set) {
-    userdata->nonblocking_error.is_set = 0;
-    rb_tinytds_raise_error(client,
-      userdata->nonblocking_error.is_message,
-      userdata->nonblocking_error.cancel,
-      userdata->nonblocking_error.error,
-      userdata->nonblocking_error.source,
-      userdata->nonblocking_error.severity,
-      userdata->nonblocking_error.dberr,
-      userdata->nonblocking_error.oserr);
+  for (short int i = 0; i < userdata->nonblocking_errors_length; i++) {
+    tinytds_errordata error = userdata->nonblocking_errors[i];
+
+    // lookahead to drain any info messages ahead of raising error
+    if (!error.is_message) {
+      for (short int j = i; j < userdata->nonblocking_errors_length; j++) {
+        tinytds_errordata msg_error = userdata->nonblocking_errors[j];
+        if (msg_error.is_message) {
+          rb_tinytds_raise_error(client, msg_error);
+        }
+      }
+    }
+
+    rb_tinytds_raise_error(client, error);
   }
+
+  free(userdata->nonblocking_errors);
+  userdata->nonblocking_errors_length = 0;
+  userdata->nonblocking_errors_size = 0;
 }
 
 static RETCODE nogvl_dbsqlok(DBPROCESS *client) {
diff --git a/test/result_test.rb b/test/result_test.rb
index 63649d4b..e40217f4 100644
--- a/test/result_test.rb
+++ b/test/result_test.rb
@@ -652,6 +652,48 @@ class ResultTest < TinyTds::TestCase
             assert_equal 1, messages.length, 'there should be one message after one print statement'
             assert_equal msg, m.message, 'message text'
           end
+
+          it 'must raise an error preceded by a `print` message' do
+            messages.clear
+            action = lambda { @client.execute("EXEC tinytds_TestPrintWithError").do }
+            assert_raise_tinytds_error(action) do |e|
+              assert_equal 'hello', messages.first.message, 'message text'
+
+              assert_equal "Error following print", e.message
+              assert_equal 16, e.severity
+              assert_equal 50000, e.db_error_number
+            end
+          end
+
+          it 'calls the provided message handler for each of a series of `print` messages' do
+            messages.clear
+            @client.execute("EXEC tinytds_TestSeveralPrints").do
+            assert_equal ['hello 1', 'hello 2', 'hello 3'], messages.map { |e| e.message }, 'message list'
+          end
+
+          it 'should flush info messages before raising error in cases of timeout' do
+            @client = new_connection timeout: 1, message_handler: Proc.new { |m| messages << m }
+            action = lambda { @client.execute("print 'hello'; waitfor delay '00:00:02'").do }
+            messages.clear
+            assert_raise_tinytds_error(action) do |e|
+              assert_match %r{timed out}i, e.message, 'ignore if non-english test run'
+              assert_equal 6, e.severity
+              assert_equal 20003, e.db_error_number
+              assert_equal 'hello', messages.first&.message, 'message text'
+            end
+          end
+
+          it 'should print info messages before raising error in cases of timeout' do
+            @client = new_connection timeout: 1, message_handler: Proc.new { |m| messages << m }
+            action = lambda { @client.execute("raiserror('hello', 1, 1) with nowait; waitfor delay '00:00:02'").do }
+            messages.clear
+            assert_raise_tinytds_error(action) do |e|
+              assert_match %r{timed out}i, e.message, 'ignore if non-english test run'
+              assert_equal 6, e.severity
+              assert_equal 20003, e.db_error_number
+              assert_equal 'hello', messages.first&.message, 'message text'
+            end
+          end
         end
 
         it 'must not raise an error when severity is 10 or less' do
@@ -770,4 +812,3 @@ def insert_and_select_datatype(datatype)
   end
 
 end
-
diff --git a/test/test_helper.rb b/test/test_helper.rb
index fc364b42..66f56d1c 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -154,6 +154,8 @@ def load_current_schema
       loader.execute(drop_sql).do
       loader.execute(schema_sql).do
       loader.execute(sp_sql).do
+      loader.execute(sp_error_sql).do
+      loader.execute(sp_several_prints_sql).do
       loader.close
       true
     end
@@ -168,7 +170,16 @@ def drop_sql_sybase
         ) DROP TABLE datatypes
         IF EXISTS(
           SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestReturnCodes'
-        ) DROP PROCEDURE tinytds_TestReturnCodes|
+        ) DROP PROCEDURE tinytds_TestReturnCodes
+        IF EXISTS(
+          SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestPrintWithError'
+        ) DROP PROCEDURE tinytds_TestPrintWithError
+        IF EXISTS(
+          SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestPrintWithError'
+        ) DROP PROCEDURE tinytds_TestPrintWithError
+        IF EXISTS(
+          SELECT 1 FROM sysobjects WHERE type = 'P' AND name = 'tinytds_TestSeveralPrints'
+        ) DROP PROCEDURE tinytds_TestSeveralPrints|
     end
 
     def drop_sql_microsoft
@@ -182,7 +193,15 @@ def drop_sql_microsoft
         IF EXISTS (
           SELECT name FROM sysobjects
           WHERE name = 'tinytds_TestReturnCodes' AND type = 'P'
-        ) DROP PROCEDURE tinytds_TestReturnCodes|
+        ) DROP PROCEDURE tinytds_TestReturnCodes
+        IF EXISTS (
+          SELECT name FROM sysobjects
+          WHERE name = 'tinytds_TestPrintWithError' AND type = 'P'
+        ) DROP PROCEDURE tinytds_TestPrintWithError
+        IF EXISTS (
+          SELECT name FROM sysobjects
+          WHERE name = 'tinytds_TestSeveralPrints' AND type = 'P'
+        ) DROP PROCEDURE tinytds_TestSeveralPrints|
     end
 
     def sp_sql
@@ -192,6 +211,21 @@ def sp_sql
         RETURN(420) |
     end
 
+    def sp_error_sql
+      %|CREATE PROCEDURE tinytds_TestPrintWithError
+        AS
+        PRINT 'hello'
+        RAISERROR('Error following print', 16, 1)|
+    end
+
+    def sp_several_prints_sql
+      %|CREATE PROCEDURE tinytds_TestSeveralPrints
+        AS
+        PRINT 'hello 1'
+        PRINT 'hello 2'
+        PRINT 'hello 3'|
+    end
+
     def find_value(id, column, query_options={})
       query_options[:timezone] ||= :utc
       sql = "SELECT [#{column}] FROM [datatypes] WHERE [id] = #{id}"

From 3202303a4b1fc2ca4800e035712ca74e0fea1634 Mon Sep 17 00:00:00 2001
From: aharpervc <aharper@breuer.com>
Date: Tue, 19 Jan 2021 14:31:09 -0500
Subject: [PATCH 4/5] wip: 2.1.4-pre

---
 CHANGELOG.md | 3 +++
 VERSION      | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ed76d23b..f1ab10b9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 ## (unreleased)
 
+## 2.1.4
+
+* Raise error if client is unable to execute statement
 * Improve handling of network related timeouts
 * Fix error reporting when preceeded by info message
 
diff --git a/VERSION b/VERSION
index ac2cdeba..f4ef8be3 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.1.3
+2.1.4.pre2

From fab9633dec258ae1081cb0eb9222b361511349c0 Mon Sep 17 00:00:00 2001
From: Wanderson Policarpo <wpolicarpo@gmail.com>
Date: Tue, 6 Apr 2021 23:42:19 +0100
Subject: [PATCH 5/5] Add test case for when connection is dead and tiny_tds
 raises an error

---
 ext/tiny_tds/client.c |  4 ++--
 test/client_test.rb   | 34 +++++++++++++++++++++++-----------
 test/test_helper.rb   |  4 ----
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/ext/tiny_tds/client.c b/ext/tiny_tds/client.c
index 0756cd03..c24c026d 100644
--- a/ext/tiny_tds/client.c
+++ b/ext/tiny_tds/client.c
@@ -295,9 +295,9 @@ static VALUE rb_tinytds_execute(VALUE self, VALUE sql) {
   rb_tinytds_client_reset_userdata(cwrap->userdata);
   REQUIRE_OPEN_CLIENT(cwrap);
   dbcmd(cwrap->client, StringValueCStr(sql));
-  if (dbsqlsend(cwrap->client) == FAIL) {
+  if (dbdead(cwrap->client) || dbsqlsend(cwrap->client) == FAIL) {
     rb_raise(cTinyTdsError, "failed to execute statement");
-    return self;
+    return Qnil;
   }
   cwrap->userdata->dbsql_sent = 1;
   result = rb_tinytds_new_result_obj(cwrap);
diff --git a/test/client_test.rb b/test/client_test.rb
index 15a0421e..6cb99efa 100644
--- a/test/client_test.rb
+++ b/test/client_test.rb
@@ -84,16 +84,9 @@ class ClientTest < TinyTds::TestCase
       options = connection_options :login_timeout => 1, :dataserver => 'DOESNOTEXIST'
       action = lambda { new_connection(options) }
       assert_raise_tinytds_error(action) do |e|
-        # Not sure why tese are different.
-        if ruby_darwin?
-          assert_equal 20009, e.db_error_number
-          assert_equal 9, e.severity
-          assert_match %r{is unavailable or does not exist}i, e.message, 'ignore if non-english test run'
-        else
-          assert_equal 20012, e.db_error_number
-          assert_equal 2, e.severity
-          assert_match %r{server name not found in configuration files}i, e.message, 'ignore if non-english test run'
-        end
+        assert_equal 20012, e.db_error_number
+        assert_equal 2, e.severity
+        assert_match %r{server name not found in configuration files}i, e.message, 'ignore if non-english test run'
       end
       assert_new_connections_work
     end
@@ -132,6 +125,26 @@ class ClientTest < TinyTds::TestCase
       end
     end
 
+    it 'raises TinyTds exception when tcp socket is down' do
+      skip if ENV['CI'] && ENV['APPVEYOR_BUILD_FOLDER'] # only CI using docker
+      begin
+        client = new_connection timeout: 2, port: 1234
+        assert_client_works(client)
+        action = lambda { client.execute("waitfor delay '00:00:01'").do }
+
+        # Use toxiproxy to close the TCP socket immediately.
+        # We want TinyTds to fail to execute the statement and raise an error immediately.
+        Toxiproxy[:sqlserver_test].down do
+          assert_raise_tinytds_error(action) do |e|
+            assert_match %r{failed to execute statement}i, e.message, 'ignore if non-english test run'
+          end
+        end
+        sleep 1
+      ensure
+        assert_new_connections_work
+      end
+    end
+
     it 'raises TinyTds exception with tcp socket network failure' do
       skip if ENV['CI'] && ENV['APPVEYOR_BUILD_FOLDER'] # only CI using docker
       begin
@@ -187,7 +200,6 @@ class ClientTest < TinyTds::TestCase
       end
       assert_new_connections_work
     end
-
   end
 
   describe '#parse_username' do
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 66f56d1c..50958e2e 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -143,10 +143,6 @@ def ruby_windows?
       RbConfig::CONFIG['host_os'] =~ /ming/
     end
 
-    def ruby_darwin?
-      RbConfig::CONFIG['host_os'] =~ /darwin/
-    end
-
     def load_current_schema
       loader = new_connection
       schema_file = File.expand_path File.join(File.dirname(__FILE__), 'schema', "#{current_schema}.sql")