hscan_match no longer works on async connections

[rukai]

hscan_match (possibly other commands, I havent checked) no longer returns any items on an async connection in redis-rs 0.22, it worked fine in redis-rs 0.21

I modified the test_filtered_scanning test to reproduce the issue:

#[tokio::test(flavor = "multi_thread")]
async fn test_filtered_scanning() {
    use redis::AsyncCommands;

    let ctx = TestContext::new();
    let mut con = ctx.async_connection().await.unwrap();
    let mut unseen = HashSet::new();

    for x in 0..3000 {
        let _: () = con
            .hset::<_, _, _, ()>("foo", format!("key_{}_{}", x % 100, x), x)
            .await
            .unwrap();
        if x % 100 == 0 {
            unseen.insert(x);
        }
    }

    let mut iter = con.hscan_match("foo", "key_0_*").await.unwrap();

    while let Some(x) = iter.next_item().await {
        unseen.remove(&x);
    }

    assert_eq!(unseen.len(), 0); // this panics because unseen has 30 elements.
}

[jaymell]

Thanks for the report, looking into it.

[jaymell]

Ok, having looked into this further, the issue is that the return type is invalid -- the test_filtered_scanning test you reference shows the proper return value in the call to the command. This worked previously due to the prior default implementation of FromRedisValue::from_redis_values, which was changed in the latest release to no longer filter out return-values that failed conversion. Note that the test also changed in the new release.

Apologies for any confusion here. This could have been documented a bit better, and the error handling in the AsyncIter is less than ideal in that you really get no clear indication that an error was encountered. Nevertheless I don't think the particular issue here reported is a bug.

[rukai]

Thanks for the investigation!

For anyone elses reference, this means that async was irrelevant and this was all that was needed to reproduce the failure:

#[test]
fn test_filtered_scanning() {
    let ctx = TestContext::new();
    let mut con = ctx.connection();
    let mut unseen = HashSet::new();

    for x in 0..3000 {
        let _: () = con
            .hset("foo", format!("key_{}_{}", x % 100, x), x)
            .unwrap();
        if x % 100 == 0 {
            unseen.insert(x);
        }
    }

    let iter = con
        .hscan_match::<&str, &str, usize>("foo", "key_0_*")
        .unwrap();

    for value in iter {
        unseen.remove(&value);
    }

    assert_eq!(unseen.len(), 0);
}

The fix was:

    let iter = con
        .hscan_match::<&str, &str, (String, usize)>("foo", "key_0_*")
        .unwrap();
        
    for (_field, value) in iter {
        unseen.remove(&value);
    }

I do find it concerning that redis-rs would just return nothing here rather than give some kind of error.
But if thats the intended design then oh well.

I'm going to leave this open in case you want to document this state of affairs at all, otherwise feel free to close this issue.

[djc]

@jaymell do you remember why we didn't start erroring out here? @rukai if you have a chance to look at the code to see about making this better, that would be great! I think #608 is what changed the behavior here.

[jaymell]

I think the core issue is the way our iterators bury errors. There's no direct way to tell if a particular command fails or a return value fails parsing other than the iterator returning None. There's definitely room for improvement here.