Skip to content

reply.go sliceHelper does not handle Redis errors within the slice #579

@mitchsw

Description

@mitchsw

I have the following line of code:

values, err := redis.ByteSlices(conn.Do("MGET", params...))

Today I investigated the following error:

redigo: unexpected element type for ByteSlices, got type redis.Error

This error was very surprising! It comes from here, which suggests that the conn.Do() reply was indeed a interface{}[], however one of the elements in the interface slice was a redis.Error.

This is technically possible based on RESP. You can have a RESP array where one of the elements is a RESP error. In redigo, this would hit this readReply for the array, and this readReply for the inner error. Such a response is poorly handled by sliceHelper.

This likely hasn't been seen before because a normal MGET etc would never reply with a slice where one element is an error. However, when using the Envoy Redis proxy with a partitioned cluster, this is possible if one of the endpoints is unavailable. I have quickly reproduced this with:

Redis partition 1/3:

127.0.0.1:6379> CLIENT PAUSE 60000 ALL
OK

Envoy Redis proxy:

127.0.0.1:6379> MGET a b c d e f g h i j k l m n o p
 1) (nil)
 2) (nil)
 3) (nil)
 4) (nil)
 5) (error) upstream failure
 6) (error) upstream failure
 7) (error) upstream failure
 8) (error) upstream failure
 9) (nil)
10) (nil)
11) (error) upstream failure
12) (error) upstream failure
13) (nil)
14) (nil)
15) (nil)
16) (nil)
(3.60s)

This reply will trigger the strange error message I saw above.

I think redigo could better handle this failure mode by propagating at least one of the array errors to the caller, e.g.:

redigo: slice contained an error: upstream failure

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions