Don't translate non-database exceptions.

The AbstractAdapter will translate all StandardErrors generated during the course of a query into ActiveRecord::StatementInvalids. Unfortunately, it'll also mangle non-database-related errors generated in ActiveSupport::Notification callbacks after the query has successfully completed. This should prevent it from translating errors from ActiveSupport::Notifications.

3 files changed, 39 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8d900d9669..b5457e389b 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Prevent AbstractAdapter from converting exceptions from ActiveSupport::Notification
+    callbacks into ActiveRecord::StatementInvalids.
+
+    *Dennis Taylor*
+
 *   Fix eager loading/preloading association with scope including joins.
 
     Fixes #28324.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index cfe1892d78..30b29e7007 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -576,12 +576,14 @@ module ActiveRecord
             type_casted_binds: type_casted_binds,
             statement_name:    statement_name,
             connection_id:     object_id) do
+            begin
               @lock.synchronize do
                 yield
               end
+            rescue => e
+              raise translate_exception_class(e, sql)
             end
-        rescue => e
-          raise translate_exception_class(e, sql)
+          end
         end
 
         def translate_exception(exception, message)
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index a1fb6427f9..6d211d5768 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -211,6 +211,28 @@ module ActiveRecord
       end
     end
 
+    def test_exceptions_from_notifications_are_not_translated
+      original_error = RuntimeError.new("This RuntimeError shouldn't get translated")
+      subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") { raise original_error }
+      actual_error = assert_raises(RuntimeError) do
+        @connection.execute("SELECT * FROM posts")
+      end
+
+      assert_equal original_error, actual_error
+
+    ensure
+      ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
+    end
+
+    def test_other_exceptions_are_translated_to_statement_invalid
+      error = assert_raises(ActiveRecord::StatementInvalid) do
+        @connection.execute("This is a syntax error")
+      end
+
+      assert_instance_of ActiveRecord::StatementInvalid, error
+      assert_instance_of syntax_error_exception_class, error.cause
+    end
+
     def test_select_all_always_return_activerecord_result
       result = @connection.select_all "SELECT * FROM posts"
       assert result.is_a?(ActiveRecord::Result)
@@ -261,6 +283,14 @@ module ActiveRecord
         assert_not_nil error.message
       end
     end
+
+    private
+
+    def syntax_error_exception_class
+      return Mysql2::Error if defined?(Mysql2)
+      return PG::SyntaxError if defined?(PG)
+      return SQLite3::SQLException if defined?(SQLite3)
+    end
   end
 
   class AdapterForeignKeyTest < ActiveRecord::TestCase