It will never reconnect when connection has closed

Hi, in the past few years, I find a critical bug in my production env. My production env use node-redis : 2.7.1. When the network disconnect , the node-redis will run the retry_strategy that I specified，and then the node-redis connection closed.But node-redis will not reconnect to the redis server again when the network is ok. So my production env always throw an error: The connection is already closed in the error log, always. Then I have to restart my node server to fix this. In my production env , the retry_strategy is:

retry_strategy: function (options) {
        // console.log('retry_strategy', options);
        if (options.error && options.error.code === 'ECONNREFUSED') {
            // End reconnecting on a specific error and flush all commands with
            // a individual error
            return new Error('The server refused the connection');
        }
        if (options.total_retry_time > 1000 * 60 * 60) {
            // End reconnecting after a specific timeout and flush all commands
            // with a individual error
            return new Error('Retry time exhausted');
        }
        if (options.attempt > 10) {
            // End reconnecting with built in error
            return new Error('Attempt time exhausted');
        }
        // reconnect after
        return Math.min(options.attempt * 100, 3000);
    }

I doubt a very long time, why this would happended. So I dig into the source codes , the node-redis version is 2.8.0. It has this bug too. Then I find the reason:

function handle_offline_command (self, command_obj) {
    var command = command_obj.command;
    var err, msg;
    if (self.closing || !self.enable_offline_queue) {
        command = command.toUpperCase();
        if (!self.closing) {
            if (self.stream.writable) {
                msg = 'The connection is not yet established and the offline queue is deactivated.';
            } else {
                msg = 'Stream not writeable.';
            }
        } else {
            msg = 'The connection is already closed.';
        }
        err = new errorClasses.AbortError({
            message: command + " can't be processed. " + msg,
            code: 'NR_CLOSED',
            command: command
        });
        if (command_obj.args.length) {
            err.args = command_obj.args;
        }
        utils.reply_in_order(self, command_obj.callback, err);
    } else {
        debug('Queueing ' + command + ' for next server connection.');
        self.offline_queue.push(command_obj);
    }
    self.should_buffer = true;
}

When the connection is down, then the ready is false and stream is not writable. And handle_offline_command will handle this .At this time the closing is always true, so it will throw an AbortError. So I donot know why retry connection to de redis server in this scope.

I fix this code like this, and it works:

function handle_offline_command (self, command_obj) {
    var command = command_obj.command;
    var err, msg;
    if (self.closing || !self.enable_offline_queue) {
        if(self.stream == null || self.stream.destroyed){
          retry_connection(self, null);
        }else{
          self.offline_queue.push(command_obj);
        }
        // command = command.toUpperCase();
        // if (!self.closing) {
        //     if (self.stream.writable) {
        //         msg = 'The connection is not yet established and the offline queue is deactivated.';
        //     } else {
        //         msg = 'Stream not writeable.';
        //     }
        // } else {
        //     msg = 'The connection is already closed.';
        // }
        // err = new errorClasses.AbortError({
        //     message: command + " can't be processed. " + msg,
        //     code: 'NR_CLOSED',
        //     command: command
        // });
        // if (command_obj.args.length) {
        //     err.args = command_obj.args;
        // }
        // utils.reply_in_order(self, command_obj.callback, err);
    } else {
        debug('Queueing ' + command + ' for next server connection.');
        self.offline_queue.push(command_obj);
    }
    self.should_buffer = true;
}

I donot know it will has any other logic code relatively, although I fix it and it work well . So I hope you give me a reliable solution, thks.The code is a little bit messy, so I very want to rewrite your node-redis using asyn await. But I have no time in this year.

Start by tracing handle_offline_command, retry_strategy, and the reconnect path in the node-redis 2.8.0 source, using the reported network-disconnect scenario as the reproduction. Check how closing, stream state, and offline-queue handling interact. Done means a connection that temporarily loses network access reconnects when the server returns without leaving commands to fail with NR_CLOSED.