fastify / fastify-redis

Plugin to share a common Redis connection across Fastify.
MIT License
198 stars 31 forks source link

closeClient does not work, but exists in the type #176

Closed starnayuta closed 1 year ago

starnayuta commented 1 year ago

Prerequisites

Fastify version

^4.0.0

Plugin version

^6.0.0

Node.js version

18.14.0

Operating system

macOS

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

13.3

Description

Since 6.0.0 closeClient does not work, but it continues to exist in index.d.ts.

https://github.com/fastify/fastify-redis/pull/147

Steps to Reproduce

The test times out if the client is not manually closed, even if closeClient is enabled.

test('custom ioredis client that is already connected', (t) => {
  t.plan(9)
  const fastify = Fastify()
  const Redis = require('ioredis')
  const redis = new Redis({ host: 'localhost', port: 6379 })

  // use the client now, so that it is connected and ready
  redis.set('key', 'value', (err) => {
    t.error(err)
    redis.get('key', (err, val) => {
      t.error(err)
      t.equal(val, 'value')

      fastify.register(fastifyRedis, {
        client: redis,
        lazyConnect: false,
        closeClient: true
      })

      fastify.ready((err) => {
        t.error(err)
        t.equal(fastify.redis, redis)

        fastify.redis.set('key2', 'value2', (err) => {
          t.error(err)
          fastify.redis.get('key2', (err, val) => {
            t.error(err)
            t.equal(val, 'value2')

            fastify.close(function (err) {
              t.error(err)
              // fastify.redis.quit(function (err) {
              //   t.error(err)
              // })
            })
          })
        })
      })
    })
  })
})

Expected Behavior

  1. fix closeClient to work
  2. remove the closeClient type definition ( = client must be closed manually).

I prefer 1.

Please tell me which solution to take, and I will send PR.

mcollina commented 1 year ago

This works as documented in the README: if the client is set, you are on your own.

A PR to change this behavior would be awesome.

starnayuta commented 1 year ago

The fix was released in v6.1.1.