Check key type first in blocking functions

[prvyk]

Whenever Redis gets a blocking pop command, it tries to pop the value first, and if the value is of the wrong type, it aborts with a WRONGTYPE error. Note that setting the key with the wrong type afterwards (while the timeout is on), will not emit a WRONGTYPE error.

This is arguably in 'bug compatible' category - but blocking when the client may not expect it is best avoided.

[TalZaccai]

Thanks for raising this issue!

What Redis does is actually a bit more nuanced here. Let me describe BLPOP for instance - initially, it checks the keys in the order that they are given, if a key has a List value and has an item - that item gets popped. If a value is not a List and is not none - it returns an error.
So essentially, if I have a list of keys k1 (list), k2 (non-list), k3 (list) - it will only fail if k1 is empty.
For example:

Another behavioral detail to keep in mind, if we block on an empty item that gets set to a different type, the operation will continue to wait and will only be released if the type changed to the correct type and an item was found:

That being said, what I think the solution should be -
That initial lookup loop for existing items happens in CollectionItemBroker.InitializeObserver. So, we need to implement a logic where if TryGetResult indicates that the value in key is of the wrong type and return an error accordingly.

Let me know if that makes sense & if you have any follow-up questions.

[prvyk]

What Redis does is actually a bit more nuanced here. Let me describe BLPOP for instance - initially, it checks the keys in the order that they are given...
That being said, what I think the solution should be - That initial lookup loop for existing items happens in CollectionItemBroker.InitializeObserver. So, we need to implement a logic where if TryGetResult indicates that the value in key is of the wrong type and return an error accordingly.

Let me know if that makes sense & if you have any follow-up questions.

It makes perfect sense. I considered that implementation strategy, but wanted to avoid touching the event code, and didn't notice the (valid object) (invalid object) behaviour, so went for the simpler initial GetKeyType()... But this doesn't leave much choice but to change the event code.