Skip to content

Commit

Permalink
Merge branch 'main' - Thank you, Adrianna for updates to errors from …
Browse files Browse the repository at this point in the history
…PR#24
  • Loading branch information
lorint committed Feb 2, 2023
2 parents 9f60801 + b975205 commit d0b39c3
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 20 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 6 additions & 6 deletions lib/active_record/connection_adapters/trilogy_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/trilogy_adapter/backwards_ar_compatibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ def initialize(connection_class, db_config, *_args)

# A do-nothing placeholder allowing AR 7.0 to function when the Trilogy driver is not patched with PR#15:
# https://github.com/github/trilogy/pull/15
require 'trilogy'
class ::Trilogy
unless const_defined?('ClientError')
class ClientError < ::StandardError
Expand Down
6 changes: 4 additions & 2 deletions lib/trilogy_adapter/lost_connection_exception_translator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
50 changes: 42 additions & 8 deletions test/lib/active_record/connection_adapters/trilogy_adapter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 }

Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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;"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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
)
Expand All @@ -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
)
Expand All @@ -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
)
Expand Down

0 comments on commit d0b39c3

Please sign in to comment.