Many asynchronous Redis operations not awaited in adapters

[fjeldstad]

When testing shutdown of a Socket.io server that uses @socket.io/redis-streams-adapter, we noticed seemingly random errors logged by ioredis: Error: Connection is closed.

This is the shutdown logic we're using:

await io.close();
redis.disconnect();

Although I'm not sure if it is the root cause, it looks like the persistSession function of the adapter does not return the result from the SET command, and therefore the calling core code cannot await it (and in fact it does not attempt to). So essentially it treats the Redis operation as synchronous even though it is not. When the Redis client is closed it might happen that the SET command has not yet finished, which means the adapter state will likely not be saved and recovery will fail upon client reconnection later. It is not as unlikely as it might sound, as the XREAD that the polling loop uses blocks the Redis connection for up to 100ms, meaning that if you're unlucky, the SET has to first wait for 100ms before executing.

Essentially I think the above code translates to roughly:

redis.xread(..., "BLOCK", 100, ...); // this blocks the Redis connection for up to 100ms - note: no await
redis.set(...); // this will not finish until the XREAD is done as it is blocking - note: no await
redis.disconnect(); // immediately closing the connection, flushing the command queue with "Connection is closed" errors

When I was looking into this, I noticed that it is quite common for Redis operations to not be awaited. See for example the close function of redis-adapter, which according to its type might return a promise but in reality never returns anything: https://github.com/socketio/socket.io-redis-adapter/blob/main/lib/index.ts#L897-L943

Is not awaiting async operations a conscious design decision or a bug?

[darrachequesne]

Yes, I think that's a bug, we should have two distinct Redis clients, so that xread calls do not block publishing.