Skip to content

Commit

Permalink
Cluster: Preserve message and suberrors when raising CommandErrorColl…
Browse files Browse the repository at this point in the history
…ection

The constructor signature for CommandErrorCollection is (errors,
message). That means if you call "raise CommandErrorCollection,
'foobar'", that actually winds up setting the _errors_ field to
'foobar', _NOT_ the message. This leads to incredibly undescriptive
"Command errors were replied on any node" errors getting emitted if
there are errors using pipelining.

Fix this by:
1) explicitly constructing the error, with the arguments in the right
order
2) mapping the sub-errors into redis-rb errors too - presumably there's
an :errors attr_reader because somebody will find that useful.
  • Loading branch information
KJ Tsanaktsidis committed Nov 17, 2023
1 parent 6729fc8 commit f639f1a
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
10 changes: 9 additions & 1 deletion cluster/lib/redis/cluster/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,15 @@ def handle_errors
raise ERROR_MAPPING.fetch(node_error.class), node_error.message, node_error.backtrace
end
end
raise ERROR_MAPPING.fetch(error.class), error.message, error.backtrace
remapped_node_errors = error.errors.map do |node_key, node_error|
remapped = ERROR_MAPPING.fetch(node_error.class, node_error.class).new(node_error.message).tap do |remapped|
remapped.set_backtrace node_error.backtrace
end
[node_key, remapped]
end.to_h
raise(Redis::Cluster::CommandErrorCollection.new(remapped_node_errors, error.message).tap do |remapped|
remapped.set_backtrace error.backtrace
end)
rescue ::RedisClient::Error => error
raise ERROR_MAPPING.fetch(error.class), error.message, error.backtrace
end
Expand Down
4 changes: 3 additions & 1 deletion cluster/test/commands_on_server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ def test_bgsave
'Use BGSAVE SCHEDULE in order to schedule a BGSAVE whenever possible.'

redis_cluster_mock(bgsave: ->(*_) { "-Error #{err_msg}" }) do |redis|
assert_raises(Redis::Cluster::CommandErrorCollection, 'Command error replied on any node') do
err = assert_raises(Redis::Cluster::CommandErrorCollection, 'Command error replied on any node') do
redis.bgsave
end
assert_includes err.message, err_msg
assert_kind_of Redis::CommandError, err.errors.values.first
end
end

Expand Down

0 comments on commit f639f1a

Please sign in to comment.