RedisStore constructor throw error will cause the program to exit？

[zurmokeeper]

const Redis = require('ioredis');
const redisConfig = {
    port: REDIS_PORT,
    host: REDIS_HOST,
    db: REDIS_SERVICE_EXPORT,
    commandTimeout: 5000, 
};

const client = new Redis(redisConfig);

const { RedisStore } = require('rate-limit-redis');            

            store: new RedisStore({
                sendCommand: async (...args) => {
                    try {
                        await client.call("DEBUG", "SLEEP", 5);
                    } catch (error) {
                        console.log('sendCommand--->', error);
                    } finally {
                        return await client.call(...args);
                    }
                },
                ........
            }),

When I executed this code, the program exited directly

rate-limit-redis 4.3.1	

source code:

constructor(options: Options) {
		if (typeof options !== 'object') {
			throw new TypeError('rate-limit-redis: Error: options object is required')
		}

		if ('sendCommand' in options && !('sendCommandCluster' in options)) {
			// Normal case: wrap the sendCommand function to convert from cluster to regular
			const sendCommandFn = options.sendCommand.bind(this)
			this.sendCommand = async ({ command }: SendCommandClusterDetails) =>
				sendCommandFn(...command)
		} else if (!('sendCommand' in options) && 'sendCommandCluster' in options) {
			this.sendCommand = options.sendCommandCluster.bind(this)
		} else {
			throw new Error(
				'rate-limit-redis: Error: options must include either sendCommand or sendCommandCluster (but not both)',
			)
		}

		this.prefix = options.prefix ?? 'rl:'
		this.resetExpiryOnChange = options.resetExpiryOnChange ?? false

		// So that the script loading can occur non-blocking, this will send
		// the script to be loaded, and will capture the value within the
		// promise return. This way, if increment/get start being called before
		// the script has finished loading, it will wait until it is loaded
		// before it continues.
		this.incrementScriptSha = this.loadIncrementScript()
		this.getScriptSha = this.loadGetScript()
	}

Both this.loadIncrementScript() and this.loadGetScript() are asynchronous methods, but neither has a catch block. And even if there is a catch block in the constructor, there is no way to pass it to the outer layer?

[nfriedly]

I think this may be the same issue that #233 was aiming to solve, although there were some issues there that need to be addressed first.

[zurmokeeper]

@nfriedly

It's the same issue. Personally, I felt that the handling of PR at that time was not appropriate. It could only catch errors, but the results obtained were incorrect. I personally believe that a more reasonable approach would be to separate this.loadIncrementScript() and this.loadGetScript() from the constructor function, for example:

class RedisStore {

  constructor() {
    ......
  }

  async init() {
    this.incrementScriptSha = await this.loadIncrementScript();
    this.getScriptSha = await this.loadGetScript();
  }
}

The operation of this init can be manually invoked by the upper layer, so that errors can be caught by the outer layer.

[nfriedly]

@zurmokeeper I put this together last night, before seeing your comment: #240

That's a good point that it probably makes more sense in an init function. express-rate-limit isn't currently set to handle async init, but we could potentially make it work. In the meanwhile, the above PR handles failures in rate-limit-redis.