From 07e7849d65e91ac5499f199944480cb3b31abfae Mon Sep 17 00:00:00 2001 From: KJ Tsanaktsidis Date: Fri, 17 Nov 2023 15:05:20 +1100 Subject: [PATCH] Cluster: Preserve message and suberrors when raising CommandErrorCollection 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. --- cluster/lib/redis/cluster/client.rb | 9 ++++++++- cluster/test/commands_on_server_test.rb | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cluster/lib/redis/cluster/client.rb b/cluster/lib/redis/cluster/client.rb index 461a8d4cc..70367c1f7 100644 --- a/cluster/lib/redis/cluster/client.rb +++ b/cluster/lib/redis/cluster/client.rb @@ -84,7 +84,14 @@ 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) + remapped.set_backtrace node_error.backtrace + [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 diff --git a/cluster/test/commands_on_server_test.rb b/cluster/test/commands_on_server_test.rb index 9bf158f5e..3fc36a7a1 100644 --- a/cluster/test/commands_on_server_test.rb +++ b/cluster/test/commands_on_server_test.rb @@ -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