-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
for pub/sub, treat length-one arrays comparably to simple values #2118
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but if we can not recurse I think it'd be a bit simpler. Probably a reason I'm not seeing through 👍
parsed = value.AsRedisValue(); | ||
return true; | ||
case ResultType.MultiBulk when value.ItemsCount == 1: | ||
return TryGetPubSubPayload(in value[0], out parsed, allowArraySingleton: false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I think this would be simpler to take the first element as a RedisValue instead of recursion just for ease of understanding/maintenance, but there may be a future case of nested arrays I'm unaware of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NickCraver yah, avoiding edge cases and avoiding having to repeat the "what combinations are allowed" (the switch
); ultimately the array here was unexpected, so... yeah: I don't want to trust anything
Well done , it's work ! |
Great question. I honestly don't know. If there is, the library will now silently ignore them (since it doesn't know what to do with them) rather than outright dying, which is... better. |
Can you please share the dlls or nuget package? |
The myget feed is here: https://www.myget.org/F/stackoverflow/api/v3/index.json - we'll push it to nuget when we're ready |
fix #2117
it appears that some of the RESP2 fallback support for client-side caching uses multi-bulk payloads; this currently causes a connection break, so we definitely need to stop the failure, but we can also interpret unary arrays similarly to simple values, which should at least allow simple cases to work