Rails 6.1: Raise ActiveRecord::ConnectionNotEstablished on calls to execute with a disconnected connection. (#903)

* raise ActiveRecord::ConnectionNotEstablished instead of ActiveRecord::StatementInvalid when call execute and connection is disconnected

* add test of other calls to connection.execute that should fail

* add changelog item

* Refactor

* fix typo

* check connection active to prevent usage of dead/closed connection

* raise TinyTds::Error and translate it. 'active?' method is rescuing just those

* Revert "check connection active to prevent usage of dead/closed connection"

This reverts commit 6861763.

* check TinyTds FalseClass response in execute_procedure and raw_connection_run

* raise ConnectionNotEstabilished when TinyTDS returns false

Co-authored-by: Aidan Haran <aharan@fusioneer.com>

Author: mgrunberg

CHANGELOG.md

@@ -16,6 +16,7 @@
 - [#892](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/892) Add support for if_exists on remove_column
 - [#883](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/885) Fix quoting of ActiveRecord::Relation::QueryAttribute and ActiveModel::Attributes
 - [#893](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/893) Add Active Record Marshal forward compatibility tests
+- [#903](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/903) Raise ActiveRecord::ConnectionNotEstablished on calls to execute with a disconnected connection
 
 #### Changed
 

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -167,7 +167,7 @@ def execute_procedure(proc_name, *variables)
           log(sql, name) do
             case @connection_options[:mode]
             when :dblib
-              result = @connection.execute(sql)
+              result = ensure_established_connection! { dblib_execute(sql) }
               options = { as: :hash, cache_rows: true, timezone: ActiveRecord::Base.default_timezone || :utc }
               result.each(options) do |row|
                 r = row.with_indifferent_access
@@ -357,13 +357,7 @@ def sp_executesql_sql(sql, types, params, name)
         def raw_connection_do(sql)
           case @connection_options[:mode]
           when :dblib
-            result = @connection.execute(sql)
-
-            # TinyTDS returns false instead of raising an exception if connection fails.
-            # Getting around this by raising an exception ourselves while this PR
-            # https://github.com/rails-sqlserver/tiny_tds/pull/469 is not released.
-            raise TinyTds::Error, "failed to execute statement" if result.is_a?(FalseClass)
-
+            result = ensure_established_connection! { dblib_execute(sql) }
             result.do
           end
         ensure
@@ -428,7 +422,7 @@ def _raw_select(sql, options = {})
         def raw_connection_run(sql)
           case @connection_options[:mode]
           when :dblib
-            @connection.execute(sql)
+            ensure_established_connection! { dblib_execute(sql) }
           end
         end
 
@@ -462,6 +456,21 @@ def finish_statement_handle(handle)
           end
           handle
         end
+
+        def dblib_execute(sql)
+          @connection.execute(sql).tap do |result|
+            # TinyTDS returns false instead of raising an exception if connection fails.
+            # Getting around this by raising an exception ourselves while this PR
+            # https://github.com/rails-sqlserver/tiny_tds/pull/469 is not released.
+            raise TinyTds::Error, "failed to execute statement" if result.is_a?(FalseClass)
+          end
+        end
+
+        def ensure_established_connection!
+          raise TinyTds::Error, 'SQL Server client is not connected' unless @connection
+
+          yield
+        end
       end
     end
   end

lib/active_record/connection_adapters/sqlserver_adapter.rb

@@ -443,6 +443,8 @@ def initialize_type_map(m = type_map)
 
       def translate_exception(e, message:, sql:, binds:)
         case message
+        when /(SQL Server client is not connected)|(failed to execute statement)/i
+          ConnectionNotEstablished.new(message)
         when /(cannot insert duplicate key .* with unique index) | (violation of unique key constraint)/i
           RecordNotUnique.new(message, sql: sql, binds: binds)
         when /(conflicted with the foreign key constraint) | (The DELETE statement conflicted with the REFERENCE constraint)/i

test/cases/disconnected_test_sqlserver.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require "cases/helper_sqlserver"
+
+class TestDisconnectedAdapter < ActiveRecord::TestCase
+  self.use_transactional_tests = false
+
+  def setup
+    @connection = ActiveRecord::Base.connection
+  end
+
+  teardown do
+    return if in_memory_db?
+    db_config = ActiveRecord::Base.connection_db_config
+    ActiveRecord::Base.establish_connection(db_config)
+  end
+
+  test "can't execute procedures while disconnected" do
+    @connection.execute_procedure :sp_tables, "sst_datatypes"
+    @connection.disconnect!
+    assert_raises(ActiveRecord::ConnectionNotEstablished, 'SQL Server client is not connected') do
+      @connection.execute_procedure :sp_tables, "sst_datatypes"
+    end
+  end
+
+  test "can't execute query while disconnected" do
+    sql = "SELECT count(*) from products WHERE id IN(@0, @1)"
+    binds = [
+      ActiveRecord::Relation::QueryAttribute.new("id", 2, ActiveRecord::Type::BigInteger.new),
+      ActiveRecord::Relation::QueryAttribute.new("id", 2, ActiveRecord::Type::BigInteger.new)
+    ]
+
+    @connection.exec_query sql, "TEST", binds
+    @connection.disconnect!
+    assert_raises(ActiveRecord::ConnectionNotEstablished, 'SQL Server client is not connected') do
+      @connection.exec_query sql, "TEST", binds
+    end
+  end
+end