Skip to content

Commit 649cb66

Browse files
underwoo16byroot
authored andcommitted
wire up retry attempt earlier to handle errors during connection prelude
1 parent 72c72d4 commit 649cb66

File tree

4 files changed

+44
-5
lines changed

4 files changed

+44
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Unreleased
22

3+
- Fix a few corner cases where `RedisClient::Error#final?` was innacurate.
34
- hiredis-client: Properly reconnect to the new leader after a sentinel failover.
45

56
# 0.26.0

lib/redis_client.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -736,8 +736,7 @@ def ensure_connected(retryable: true)
736736
connection = nil
737737
preferred_error = nil
738738
begin
739-
connection = raw_connection
740-
connection.retry_attempt = config.retriable?(tries) ? tries : nil
739+
connection = raw_connection(config.retriable?(tries) ? tries : nil)
741740
if block_given?
742741
yield connection
743742
else
@@ -776,14 +775,15 @@ def ensure_connected(retryable: true)
776775
end
777776
end
778777

779-
def raw_connection
778+
def raw_connection(retry_attempt = nil)
780779
if @raw_connection.nil? || !@raw_connection.revalidate
781-
connect
780+
connect(retry_attempt)
782781
end
782+
@raw_connection.retry_attempt = retry_attempt
783783
@raw_connection
784784
end
785785

786-
def connect
786+
def connect(retry_attempt = nil)
787787
@pid = PIDCache.pid
788788

789789
if @raw_connection&.revalidate
@@ -799,6 +799,7 @@ def connect
799799
write_timeout: write_timeout,
800800
)
801801
end
802+
@raw_connection.retry_attempt = retry_attempt
802803
end
803804

804805
prelude = config.connection_prelude.dup

lib/redis_client/ruby_connection.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ def write(command)
7373
@io.write(buffer)
7474
rescue SystemCallError, IOError, OpenSSL::SSL::SSLError => error
7575
raise connection_error(error.message)
76+
rescue Error => error
77+
error._set_config(config)
78+
error._set_retry_attempt(@retry_attempt)
79+
raise error
7680
end
7781
end
7882

@@ -85,6 +89,10 @@ def write_multi(commands)
8589
@io.write(buffer)
8690
rescue SystemCallError, IOError, OpenSSL::SSL::SSLError => error
8791
raise connection_error(error.message)
92+
rescue Error => error
93+
error._set_config(config)
94+
error._set_retry_attempt(@retry_attempt)
95+
raise error
8896
end
8997
end
9098

@@ -98,6 +106,10 @@ def read(timeout = nil)
98106
raise RedisClient::ProtocolError.with_config(error.message, config)
99107
rescue SystemCallError, IOError, OpenSSL::SSL::SSLError => error
100108
raise connection_error(error.message)
109+
rescue Error => error
110+
error._set_config(config)
111+
error._set_retry_attempt(@retry_attempt)
112+
raise error
101113
end
102114

103115
def measure_round_trip_delay

test/redis_client/middlewares_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,31 @@ def test_final_errors
129129
TestMiddleware.calls.clear
130130
end
131131

132+
def test_final_errors_during_reconnect
133+
client = new_client(reconnect_attempts: 1)
134+
simulate_network_errors(client, ["PING", "HELLO"]) do
135+
assert_raises ConnectionError do
136+
client.call("PING")
137+
end
138+
end
139+
140+
calls = TestMiddleware.calls.select { |type, _| type == :call }
141+
assert_equal 1, calls.size
142+
143+
call = calls[0]
144+
assert_equal :error, call[1]
145+
assert_equal ["PING"], call[2]
146+
refute_predicate call[3], :final?
147+
148+
pipeline_calls = TestMiddleware.calls.select { |type, _| type == :pipeline }
149+
assert_equal 2, pipeline_calls.size
150+
151+
failing_pipeline = pipeline_calls[1]
152+
assert_equal :error, failing_pipeline[1]
153+
assert_equal [["HELLO", "3"]], failing_pipeline[2]
154+
assert_predicate failing_pipeline[3], :final?
155+
end
156+
132157
module DummyMiddleware
133158
def call(command, _config, &_)
134159
command

0 commit comments

Comments
 (0)