Asynchronous PSUBSCRIBE command fails when using RESP3

[carlosabalde]

The following toy implementation shows how execution of asynchronous PSUBSCRIBE command using current RESP3 support in master (i.e. #805 already merged) fails with error Protocol error, got ">" as reply type byte. This has been tested using Redis Server 6.0.3. Same logic using RESP2 works as expected.

// gcc test.c -o test -Wall -lhiredis -lev
// redis-server --port 6400

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <assert.h>
#include <hiredis/hiredis.h>
#include <hiredis/async.h>
#include <hiredis/adapters/libev.h>

#define HOST "127.0.0.1"
#define PORT 6400
#define PROTOCOL 3

static void
connectCallback(const redisAsyncContext *ctx, int status)
{
    if (status != REDIS_OK) {
        printf("CONNECT CB: %s\n", ctx->err ? ctx->errstr : "-");
        exit(1);
    }
}

static void
disconnectCallback(const redisAsyncContext *ctx, int status)
{
    if (status != REDIS_OK) {
        printf("DISCONNECT CB: %s\n", ctx->err ? ctx->errstr : "-");
        exit(1);
    }
}

static void
helloCallback(redisAsyncContext *context, void *r, void *s)
{
    redisReply *reply = r;
    assert(
        reply != NULL &&
        (reply->type == REDIS_REPLY_ARRAY ||
         reply->type == REDIS_REPLY_MAP));
    printf("HELLO CB: %d\n", reply->type);
}

static void
psubscribeCallback(redisAsyncContext *context, void *r, void *s)
{
    redisReply *reply = r;
    if (reply != NULL) {
        printf("PSUBSCRIBE CB: %d\n", reply->type);
    }
}

static redisAsyncContext *
connect(unsigned version, struct ev_loop *loop)
{
    redisAsyncContext *ctx = redisAsyncConnect(HOST, PORT);
    assert(ctx != NULL);
    if (ctx->err) {
        printf("CONNECT: %s\n", ctx->err ? ctx->errstr : "-");
        exit(1);
    }

    redisLibevAttach(loop, ctx);
    redisAsyncSetConnectCallback(ctx, connectCallback);
    redisAsyncSetDisconnectCallback(ctx, disconnectCallback);

    if (redisAsyncCommand(ctx, helloCallback, NULL, "HELLO %d", version) != REDIS_OK) {
        if (ctx->err) {
            printf("HELLO: %s\n", ctx->err ? ctx->errstr : "-");
        }
        exit(1);
    }

    if (redisAsyncCommand(ctx, psubscribeCallback, NULL, "PSUBSCRIBE foo bar") != REDIS_OK) {
        if (ctx->err) {
            printf("PSUBSCRIBE: %s\n", ctx->err ? ctx->errstr : "-");
        }
        exit(1);
    }

    return ctx;
}

int
main(int argc, char *argv[])
{
    struct ev_loop *loop = ev_loop_new(EVFLAG_AUTO);
    assert(loop != NULL);
    assert(connect(PROTOCOL, loop) != NULL);
    while (1) {
        ev_loop(loop, EVRUN_NOWAIT);
        usleep(500000);
    }
}

[michael-grunder]

Thanks, I'll take a look.

Actually I think > is RESP3 for out-of-band "push" notifications. This might be a non-trivial change.

We should probably do it but it will warrant discussion around whether it's out of scope for a "minimalist" Redis library.

Edit: The bug also exists in redis-cli on current unstable:

$ redis-cli 
127.0.0.1:6379> hello 3
1# "server" => "redis"
2# "version" => "999.999.999"
3# "proto" => (integer) 3
4# "id" => (integer) 5
5# "mode" => "standalone"
6# "role" => "master"
7# "modules" => (empty array)
127.0.0.1:6379> subscribe foo
Reading messages... (press Ctrl-C to quit)
Error: Protocol error, got ">" as reply type byte

[michael-grunder]

Turned out to be simpler than I thought.

Link to the branch on my fork

[carlosabalde]

I've tested your branch in the project I'm working on and I confirm it fixes the issue. Thanks a lot! :)