Merge pull request trilogy-libraries#24 from adrianna-chang-shopify/ac-translate-new-error-classes

Translate new error classes

Author: HParker

Gemfile

@@ -8,4 +8,6 @@ else
   gem "activerecord", ENV["RAILS_VERSION"]
 end
 
+gem "trilogy", git: "https://github.com/github/trilogy", branch: "main", glob: "contrib/ruby/*.gemspec"
+
 gemspec

lib/active_record/connection_adapters/trilogy_adapter.rb

@@ -56,8 +56,6 @@ def last_inserted_id(result)
 
       ER_BAD_DB_ERROR = 1049
       ER_ACCESS_DENIED_ERROR = 1045
-      ER_CONN_HOST_ERROR = 2003
-      ER_UNKNOWN_HOST_ERROR = 2005
 
       ADAPTER_NAME = "Trilogy"
 
@@ -75,7 +73,7 @@ class << self
         def new_client(config)
           config[:ssl_mode] = parse_ssl_mode(config[:ssl_mode]) if config[:ssl_mode]
           ::Trilogy.new(config)
-        rescue Trilogy::DatabaseError => error
+        rescue Trilogy::ConnectionError, Trilogy::ProtocolError => error
           raise translate_connect_error(config, error)
         end
 
@@ -95,10 +93,12 @@ def translate_connect_error(config, error)
             ActiveRecord::NoDatabaseError.db_error(config[:database])
           when ER_ACCESS_DENIED_ERROR
             ActiveRecord::DatabaseConnectionError.username_error(config[:username])
-          when ER_CONN_HOST_ERROR, ER_UNKNOWN_HOST_ERROR
-            ActiveRecord::DatabaseConnectionError.hostname_error(config[:host])
           else
-            ActiveRecord::ConnectionNotEstablished.new(error.message)
+            if error.message.match?(/TRILOGY_DNS_ERROR/)
+              ActiveRecord::DatabaseConnectionError.hostname_error(config[:host])
+            else
+              ActiveRecord::ConnectionNotEstablished.new(error.message)
+            end
           end
         end
       end

lib/trilogy_adapter/lost_connection_exception_translator.rb

@@ -37,8 +37,10 @@ def translate_ruby_exception
           Errors::BrokenPipe.new(message)
         when SocketError, IOError
           Errors::SocketError.new(message)
-        when Errno::ECONNRESET
-          Errors::ConnectionResetByPeer.new(message)
+        when Trilogy::ConnectionError
+          if message.match?(/Connection reset by peer/)
+            Errors::ConnectionResetByPeer.new(message)
+          end
         end
       end
 

test/lib/active_record/connection_adapters/trilogy_adapter_test.rb

@@ -27,6 +27,40 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
     @adapter.disconnect!
   end
 
+  test ".new_client" do
+    client = @adapter.class.new_client(@configuration)
+    assert_equal Trilogy, client.class
+  end
+
+  test ".new_client on db error" do
+    configuration = @configuration.merge(database: "unknown")
+    assert_raises ActiveRecord::NoDatabaseError do
+      @adapter.class.new_client(configuration)
+    end
+  end
+
+  test ".new_client on access denied error" do
+    skip("Test fails intermittently with TRILOGY_PROTOCOL_VIOLATION. See https://github.com/github/trilogy/pull/42")
+    configuration = @configuration.merge(username: "unknown")
+    assert_raises ActiveRecord::DatabaseConnectionError do
+      @adapter.class.new_client(configuration)
+    end
+  end
+
+  test ".new_client on host error" do
+    configuration = @configuration.merge(host: "unknown")
+    assert_raises ActiveRecord::DatabaseConnectionError do
+      @adapter.class.new_client(configuration)
+    end
+  end
+
+  test ".new_client on port error" do
+    configuration = @configuration.merge(port: 1234)
+    assert_raises ActiveRecord::ConnectionNotEstablished do
+      @adapter.class.new_client(configuration)
+    end
+  end
+
   test "#explain for one query" do
     explain = @adapter.explain("select * from posts")
     assert_match %(possible_keys), explain
@@ -121,7 +155,7 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
   end
 
   test "#active? answers false with connection and exception" do
-    @adapter.send(:connection).stub(:ping, -> { raise Trilogy::Error.new }) do
+    @adapter.send(:connection).stub(:ping, -> { raise Trilogy::BaseError.new }) do
       assert_equal false, @adapter.active?
     end
   end
@@ -147,11 +181,11 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
     adapter = trilogy_adapter_with_connection(old_connection)
 
     begin
-      Trilogy.stub(:new, -> _ { raise Trilogy::Error.new }) do
+      Trilogy.stub(:new, -> _ { raise Trilogy::BaseError.new }) do
         adapter.reconnect!
       end
     rescue ActiveRecord::StatementInvalid => ex
-      assert_instance_of Trilogy::Error, ex.cause
+      assert_instance_of Trilogy::BaseError, ex.cause
     else
       flunk "Expected Trilogy::Error to be raised"
     end
@@ -381,7 +415,7 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
 
     # Cause an ER_SERVER_SHUTDOWN error (code 1053) after the session is
     # set. On reconnect, the adapter will get a real, working connection.
-    server_shutdown_error = Trilogy::DatabaseError.new
+    server_shutdown_error = Trilogy::ProtocolError.new
     server_shutdown_error.instance_variable_set(:@error_code, 1053)
     mock_connection.expect(:query, nil) { raise server_shutdown_error }
 
@@ -416,7 +450,7 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
     assert adapter.active?
 
     # Make connection lost for future queries by exceeding the read timeout
-    assert_raises(Errno::ETIMEDOUT) do
+    assert_raises(Trilogy::TimeoutError) do
       connection.query "SELECT sleep(2);"
     end
     assert_not adapter.active?
@@ -497,7 +531,7 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
       adapter.execute("SELECT 1")
 
       # Cause the client to disconnect without the adapter's awareness
-      assert_raises Errno::ETIMEDOUT do
+      assert_raises Trilogy::TimeoutError do
         adapter.send(:connection).query("SELECT sleep(2)")
       end
 
@@ -519,7 +553,7 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
         adapter.execute("SELECT 1")
 
         # Cause the client to disconnect without the adapter's awareness
-        assert_raises Errno::ETIMEDOUT do
+        assert_raises Trilogy::TimeoutError do
           adapter.send(:connection).query("SELECT sleep(2)")
         end
       end
@@ -558,7 +592,7 @@ class ActiveRecord::ConnectionAdapters::TrilogyAdapterTest < TestCase
   test "#execute fails with unknown error" do
     assert_raises_with_message(ActiveRecord::StatementInvalid, /A random error/) do
       connection = Minitest::Mock.new Trilogy.new(@configuration)
-      connection.expect(:query, nil) { raise Trilogy::DatabaseError, "A random error." }
+      connection.expect(:query, nil) { raise Trilogy::ProtocolError, "A random error." }
       adapter = trilogy_adapter_with_connection(connection)
 
       adapter.execute "SELECT * FROM posts;"

test/lib/trilogy_adapter/lost_connection_exception_translator_test.rb

@@ -5,7 +5,7 @@
 class TrilogyAdapter::LostConnectionExceptionTranslatorTest < TestCase
   test "#translate returns appropriate TrilogyAdapter error for Trilogy exceptions" do
     translator = TrilogyAdapter::LostConnectionExceptionTranslator.new(
-      Trilogy::DatabaseError.new,
+      Trilogy::ProtocolError.new,
       "ER_SERVER_SHUTDOWN 1053",
       1053
     )
@@ -15,7 +15,7 @@ class TrilogyAdapter::LostConnectionExceptionTranslatorTest < TestCase
 
   test "#translate returns nil for Trilogy exceptions when the error code is not given" do
     translator = TrilogyAdapter::LostConnectionExceptionTranslator.new(
-      Trilogy::DatabaseError.new,
+      Trilogy::ProtocolError.new,
       "ER_SERVER_SHUTDOWN 1053",
       nil
     )
@@ -35,7 +35,7 @@ class TrilogyAdapter::LostConnectionExceptionTranslatorTest < TestCase
 
   test "#translate returns appropriate TrilogyAdapter error for lost connection Trilogy exceptions" do
     translator = TrilogyAdapter::LostConnectionExceptionTranslator.new(
-      Trilogy::Error.new,
+      Trilogy::BaseError.new,
       "TRILOGY_UNEXPECTED_PACKET",
       nil
     )
@@ -45,7 +45,7 @@ class TrilogyAdapter::LostConnectionExceptionTranslatorTest < TestCase
 
   test "#translate returns nil for non-lost connection exceptions" do
     translator = TrilogyAdapter::LostConnectionExceptionTranslator.new(
-      Trilogy::Error.new,
+      Trilogy::BaseError.new,
       "Something bad happened but it wasn't a lost connection so...",
       nil
     )