ERR_AVVIO_PLUGIN_TIMEOUT when supplying an existing client and lazyConnect: false

[michaelrommel]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.22.0

Plugin version

4.3.2

Node.js version

14.18.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

debian bullseye 11

Description

Hi,

the patch from July 14th seems to break the plugin, only in case we supply a pre-existing and connected redis client to the plugin and not setting lazyConnect to true.

I believe this happens because then the code part from index.js:67-103 gets executed, registers the event handlers and then calls return in line 102 without calling next(). (At least that is what I could tell from the source code.)

https://github.com/fastify/fastify-redis/blob/cc97a8e140482bcd5f06a8bbc6abc2980e3d5830/index.js#L67-L106

If I call the plugin like this:

app.register(fastifyRedis, { client: options.redis, lazyConnect: true });

the error no longer occurs.

Steps to Reproduce

1. instantiate a redis instance
2. use this redis instance to perform some operations (this ensures, that the client is connected and ready)
3. instantiate fastify
4. add the fastify-redis plugin like:

   app.register(fastifyRedis, { client: options.redis });
   or
   app.register(fastifyRedis, { client: options.redis, lazyConnect: false });

5. the ERR_AVVIO_PLUGIN_TIMEOUT occurs

Expected Behavior

Fastify starts in the same way as with lazyConnect: true.

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[michaelrommel]

I'll gladly give it a try, but I am a node.js intermediate user, no expert... Give me a day or two...

[michaelrommel]

Hi there,

I have prepared for a PR. Small question however:

I saw that explicitly in the custom client test cases the node-redis module was used to instantiate the existing client, whereas fastify-redis per default uses the ioredis module. My solution for the issue was to check first, if the client is already ready and only register the event handlers if this is not the case. Now both modules do not expose a check for readiness in the same way:

• redis uses client.ready and
• ioredis uses client.status

And version 4.0 of redis will have something like isOpen() but I have not investigated this further and could not easily see a check for ready. (But it would need to be amended again, once 4.0 is released...)

So is this the right direction to cater for different custom clients in the code? Or would you have something different in mind? Also do you think this test needs to be created similarly for all the other custom client test cases?

Please let me know your guidance.

Michael.

[mcollina]

So is this the right direction to cater for different custom clients in the code? Or would you have something different in mind? Also do you think this test needs to be created similarly for all the other custom client test cases?

It's the right direction.