redis
fix: add blockingTimeout option for blocking commands
If I have understood the code correctly, the forced timeout will only be applied if we pass the new blockingTimeout option explicitly. The main issues I see with this approach is that by default the instance can hang forever if the user uses blocking commands, as the watchdog is not enabled when we have a specific timeout argument passed in the Redis command. Also, I was not sure how this setting can be used per command. For example in BullMQ we have different timeouts when we call bzpopmin, depending on the maximum time we want the command to wait until it gives up (this is required by the delayed jobs functionality among other things).
If I have understood the code correctly, the forced timeout will only be applied if we pass the new blockingTimeout option explicitly. The main issues I see with this approach is that by default the instance can hang forever if the user uses blocking commands, as the watchdog is not enabled when we have a specific timeout argument passed in the Redis command. Also, I was not sure how this setting can be used per command. For example in BullMQ we have different timeouts when we call bzpopmin, depending on the maximum time we want the command to wait until it gives up (this is required by the delayed jobs functionality among other things).
You understood it correctly. To begin with, I’d recommend making this an opt-in feature for safety. Supporting per-command options is not straightforward because the command signatures are generated automatically in RedisCommander by this script. A builder pattern or a similar mechanism might offer a more practical way to enable per-command timeouts.
So the way to see this new option is like a fallback for this type of disconnections and nothing else. In this case I guess it is ok. I can test it in my setup to see if it solves the problem for us.
Would it be possible to have a setter for this option so that it could be changed on an existing instance?
Something is wrong with this implementation, testing with this code:
import Redis from "ioredis";
const port = 6379;
const connection = new Redis({
host: "localhost",
port,
blockingTimeout: 1000,
});
connection.on("connect", () => console.log("Redis connected!"));
connection.on("ready", () => console.log("Redis ready!"));
connection.on("error", (err) => console.error("Redis error:", err));
connection.on("end", () => console.log("Redis connection ended"));
connection.on("reconnecting", () => console.log("Redis reconnecting..."));
async function test() {
try {
console.log("Gonna issue a BZPOPMIN command...");
const start = Date.now();
const result = await connection.bzpopmin("key", 10);
console.log("Issued BZPOPMIN command!", result);
console.log(`BZPOPMIN command took ${Date.now() - start} ms`);
} catch {
console.error("Error issuing BZPOPMIN command");
}
}
test().catch(console.error);
What happens is that it throws an exception after 1 second, but then it keeps trying to reconnect and throwing the same exception, which should not happen. It should reconnect and thats it.
bun ioredis-bug.ts
Gonna issue a BZPOPMIN command...
Redis connected!
Redis ready!
Redis error: 345 | this.options.blockingTimeout > 0 &&
346 | Command_1.default.checkFlag("BLOCKING_COMMANDS", command.name)) {
347 | command.setBlockingTimeout(this.options.blockingTimeout, () => {
348 | var _a;
349 | // Destroy stream to force reconnection
350 | (_a = this.stream) === null || _a === void 0 ? void 0 : _a.destroy(new Error("Blocking command timed out - reconnecting"));
^
error: Blocking command timed out - reconnecting
at /Users/manuelastudillo/dev/ioredis/built/Redis.js:350:88
at /Users/manuelastudillo/dev/ioredis/built/Command.js:381:16
Error issuing BZPOPMIN command
Redis reconnecting...
Redis connected!
Redis ready!
Redis error: 345 | this.options.blockingTimeout > 0 &&
346 | Command_1.default.checkFlag("BLOCKING_COMMANDS", command.name)) {
347 | command.setBlockingTimeout(this.options.blockingTimeout, () => {
348 | var _a;
349 | // Destroy stream to force reconnection
350 | (_a = this.stream) === null || _a === void 0 ? void 0 : _a.destroy(new Error("Blocking command timed out - reconnecting"));
^
error: Blocking command timed out - reconnecting
at /Users/manuelastudillo/dev/ioredis/built/Redis.js:350:88
at /Users/manuelastudillo/dev/ioredis/built/Command.js:381:16
Redis reconnecting...
Redis connected!
Redis ready!
Redis error: 345 | this.options.blockingTimeout > 0 &&
346 | Command_1.default.checkFlag("BLOCKING_COMMANDS", command.name)) {
347 | command.setBlockingTimeout(this.options.blockingTimeout, () => {
348 | var _a;
349 | // Destroy stream to force reconnection
350 | (_a = this.stream) === null || _a === void 0 ? void 0 : _a.destroy(new Error("Blocking command timed out - reconnecting"));
^
error: Blocking command timed out - reconnecting
at /Users/manuelastudillo/dev/ioredis/built/Redis.js:350:88
at /Users/manuelastudillo/dev/ioredis/built/Command.js:381:16
Redis reconnecting...
Redis connected!
Redis ready!
Redis error: 345 | this.options.blockingTimeout > 0 &&
346 | Command_1.default.checkFlag("BLOCKING_COMMANDS", command.name)) {
347 | command.setBlockingTimeout(this.options.blockingTimeout, () => {
348 | var _a;
349 | // Destroy stream to force reconnection
350 | (_a = this.stream) === null || _a === void 0 ? void 0 : _a.destroy(new Error("Blocking command timed out - reconnecting"));
^
error: Blocking command timed out - reconnecting
at /Users/manuelastudillo/dev/ioredis/built/Redis.js:350:88
at /Users/manuelastudillo/dev/ioredis/built/Command.js:381:16
Redis reconnecting...
Redis connected!
Redis ready!
Redis error: 345 | this.options.blockingTimeout > 0 &&
346 | Command_1.default.checkFlag("BLOCKING_COMMANDS", command.name)) {
347 | command.setBlockingTimeout(this.options.blockingTimeout, () => {
348 | var _a;
349 | // Destroy stream to force reconnection
350 | (_a = this.stream) === null || _a === void 0 ? void 0 : _a.destroy(new Error("Blocking command timed out - reconnecting"));
^
error: Blocking command timed out - reconnecting
at /Users/manuelastudillo/dev/ioredis/built/Redis.js:350:88
at /Users/manuelastudillo/dev/ioredis/built/Command.js:381:16
Redis reconnecting...
Redis connected!
Redis ready!
^C
it is as if it is retrying the command also, but if the command timed-out we should not retry it as it just thrown an exception.
Hey @manast, thank you for taking the time to check it out.
I’ll take a look when I get a chance. If you want to explore a fix in the meantime, feel free to give it a try.