Conversation
mgravell
changed the title
NickCraver
left a comment
NickCraver
left a comment
There was a problem hiding this comment.
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.
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.
@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 |
4 participants
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