Skip to content

Commit 6616770

Browse files
Translate Trilogy syscall errors as conn failed
At GitHub we get a fair number of Trilogy `ETIMEDOUT` errors (for known reasons that we might be able to improve somewhat, but I doubt we'll make them go away entirely). These are very much retryable network errors, so it'd be handy if these `ETIMEDOUT` errors were translated to `ConnectionFailed` instead of `StatementInvalid`, making them `retryable_connection_error`s. https://github.com/rails/rails/blob/ed2bc92b82ddc111150cdf48bb646fd97b3baacb/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb#L1077 We're already translating `ECONNRESET` (via matching on the error message) and `EPIPE` to `ConnectionFailed`. Rather than adding another case, this commit treats all of the Trilogy `SystemCallError` subclasses as `ConnectionFailed`. This requires bumping trilogy 2.7 so we can get trilogy-libraries/trilogy#143
1 parent 776626f commit 6616770

File tree

6 files changed

+41
-8
lines changed

6 files changed

+41
-8
lines changed

Gemfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ platforms :ruby, :windows do
158158
group :db do
159159
gem "pg", "~> 1.3"
160160
gem "mysql2", "~> 0.5"
161-
gem "trilogy", ">= 2.5.0"
161+
gem "trilogy", ">= 2.7.0"
162162
end
163163
end
164164

Gemfile.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ GEM
543543
timeout (0.4.1)
544544
tomlrb (2.0.3)
545545
trailblazer-option (0.1.2)
546-
trilogy (2.6.0)
546+
trilogy (2.7.0)
547547
turbo-rails (1.5.0)
548548
actionpack (>= 6.0.0)
549549
activejob (>= 6.0.0)
@@ -654,7 +654,7 @@ DEPENDENCIES
654654
syntax_tree (= 6.1.1)
655655
tailwindcss-rails
656656
terser (>= 1.1.4)
657-
trilogy (>= 2.5.0)
657+
trilogy (>= 2.7.0)
658658
turbo-rails
659659
tzinfo-data
660660
useragent

activerecord/lib/active_record/connection_adapters/trilogy_adapter.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require "active_record/connection_adapters/abstract_mysql_adapter"
44

5-
gem "trilogy", "~> 2.4"
5+
gem "trilogy", "~> 2.7"
66
require "trilogy"
77

88
require "active_record/connection_adapters/trilogy/database_statements"
@@ -209,10 +209,11 @@ def translate_exception(exception, message:, sql:, binds:)
209209
end
210210

211211
case exception
212-
when Errno::EPIPE, SocketError, IOError
212+
when SocketError, IOError
213213
return ConnectionFailed.new(message, connection_pool: @pool)
214214
when ::Trilogy::Error
215-
if /Connection reset by peer|TRILOGY_CLOSED_CONNECTION|TRILOGY_INVALID_SEQUENCE_ID|TRILOGY_UNEXPECTED_PACKET/.match?(exception.message)
215+
if /TRILOGY_CLOSED_CONNECTION|TRILOGY_INVALID_SEQUENCE_ID|TRILOGY_UNEXPECTED_PACKET/.match?(exception.message) ||
216+
exception.is_a?(SystemCallError)
216217
return ConnectionFailed.new(message, connection_pool: @pool)
217218
end
218219
end

activerecord/test/cases/adapters/trilogy/trilogy_adapter_test.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,38 @@ class TrilogyAdapterTest < ActiveRecord::TrilogyTestCase
358358
assert_includes error.message, "/var/invalid.sock"
359359
end
360360

361+
test "EPIPE raises ActiveRecord::ConnectionFailed" do
362+
assert_raises(ActiveRecord::ConnectionFailed) do
363+
@conn.raw_connection.stub(:query, -> (*) { raise Trilogy::SyscallError::EPIPE }) do
364+
@conn.execute("SELECT 1")
365+
end
366+
end
367+
end
368+
369+
test "ETIMEDOUT raises ActiveRecord::ConnectionFailed" do
370+
assert_raises(ActiveRecord::ConnectionFailed) do
371+
@conn.raw_connection.stub(:query, -> (*) { raise Trilogy::SyscallError::ETIMEDOUT }) do
372+
@conn.execute("SELECT 1")
373+
end
374+
end
375+
end
376+
377+
test "ECONNREFUSED raises ActiveRecord::ConnectionFailed" do
378+
assert_raises(ActiveRecord::ConnectionFailed) do
379+
@conn.raw_connection.stub(:query, -> (*) { raise Trilogy::SyscallError::ECONNREFUSED }) do
380+
@conn.execute("SELECT 1")
381+
end
382+
end
383+
end
384+
385+
test "ECONNRESET raises ActiveRecord::ConnectionFailed" do
386+
assert_raises(ActiveRecord::ConnectionFailed) do
387+
@conn.raw_connection.stub(:query, -> (*) { raise Trilogy::SyscallError::ECONNRESET }) do
388+
@conn.execute("SELECT 1")
389+
end
390+
end
391+
end
392+
361393
# Create a temporary subscription to verify notification is sent.
362394
# Optionally verify the notification payload includes expected types.
363395
def assert_notification(notification, expected_payload = {}, &block)

railties/lib/rails/generators/database.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def initialize(*)
1414
def gem_for_database(database = options[:database])
1515
case database
1616
when "mysql" then ["mysql2", ["~> 0.5"]]
17-
when "trilogy" then ["trilogy", ["~> 2.4"]]
17+
when "trilogy" then ["trilogy", ["~> 2.7"]]
1818
when "postgresql" then ["pg", ["~> 1.1"]]
1919
when "sqlite3" then ["sqlite3", ["~> 1.4"]]
2020
when "oracle" then ["activerecord-oracle_enhanced-adapter", nil]

railties/test/generators/db_system_change_generator_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class ChangeGeneratorTest < Rails::Generators::TestCase
100100

101101
assert_file("Gemfile") do |content|
102102
assert_match "# Use trilogy as the database for Active Record", content
103-
assert_match 'gem "trilogy", "~> 2.4"', content
103+
assert_match 'gem "trilogy", "~> 2.7"', content
104104
end
105105

106106
assert_file("Dockerfile") do |content|

0 commit comments

Comments
 (0)