-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reply.go sliceHelper does not handle Redis errors within the slice #579
Comments
Seems like the |
Thanks for the reply @stevenh. I could have been clearer, sorry. The return value of
was
So no, redigo didn't propagate the It could be handled better by maybe just propagating the first error, e.g.
or some kind of annotation, e.g.
|
Thanks for clarifying @mitchsw! |
Surface the underlying error when processing results for slice and map helpers so that the user can see the real cause and not a type mismatch error. Also: * Formatting changes to improve error case separation. * Leverage %w in reply errors. Fixes: #579
Surface the underlying error when processing results for slice and map helpers so that the user can see the real cause and not a type mismatch error. Also: * Formatting changes to improve error case separation. * Leverage %w in reply errors. Fixes: #579
I have the following line of code:
Today I investigated the following 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 aredis.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:
Envoy Redis proxy:
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.:
The text was updated successfully, but these errors were encountered: