fastify / fastify-redis

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

how to handle connection error #170

Closed stefanocudini closed 1 year ago

stefanocudini commented 1 year ago

Prerequisites

Issue

what is the most correct way to handle redis connection errors, to have a simple human error log?

...
const fastify = Fastify({logger});

fastify.ready(err => {
  if (err) {
    fastify.log.debug(`Redis Connection Error ${err.code}`)
  }
})

fastify.register(require('@fastify/redis'), {
  host: config.redis.hosts,
  password: config.redis.password
});
mcollina commented 1 year ago

What error are you getting right now?

stefanocudini commented 1 year ago

simply connection error to my redis instance. I can't figure out for this error where I should put my try-catch to do these two things:

the code above shows this error on run, if redis is unreachable, but I don't understand what is the best way to catch the error

/home/stefc/app/node_modules/fastify/fastify.js:541
        ? appendStackTrace(err, new AVVIO_ERRORS_MAP[err.code](err.message))
                                ^
FastifyError [Error]: fastify-plugin: Plugin did not start in time: '@fastify/redis'. You may have forgotten to call 'done' function or to resolve a Promise

...

 {
  code: 'FST_ERR_PLUGIN_TIMEOUT',
  statusCode: 500,
  cause: AvvioError [Error]: Plugin did not start in time: '@fastify/redis'. You may have forgotten to call 'done' function or to resolve a Promise
      at Timeout._onTimeout (/home/stefc/app/node_modules/avvio/plugin.js:122:19)
      at listOnTimeout (node:internal/timers:559:17)
      at processTimers (node:internal/timers:502:7) {
    code: 'AVV_ERR_READY_TIMEOUT',
    fn: <ref *1> [Function: fastifyRedis] {
      default: [Circular *1],
      fastifyRedis: [Circular *1],
      [Symbol(skip-override)]: true,
      [Symbol(fastify.display-name)]: '@fastify/redis',
      [Symbol(plugin-meta)]: { fastify: '4.x', name: '@fastify/redis' }
    }
  }
}
[nodemon] app crashed - waiting for file changes before starting...
kibertoad commented 1 year ago

@stefanocudini Problem with redis is that it doesn't have built-in timeout functionality. You need to add it yourself. See how it is implemented using runWithTimeout here: https://github.com/lokalise/node-service-template/blob/main/src/infrastructure/healthchecks.ts

stefanocudini commented 1 year ago

@stefanocudini Problem with redis is that it doesn't have built-in timeout functionality. You need to add it yourself. See how it is implemented using runWithTimeout here: https://github.com/lokalise/node-service-template/blob/main/src/infrastructure/healthchecks.ts

but I just want to catch the error: FST_ERR_PLUGIN_TIMEOUT, how can I do this?

stefanocudini commented 1 year ago

I'm try other ways... but nothing catch the exceptions

const fastify = Fastify({logger});

fastify.setErrorHandler(function (error, request, reply) {
  fastify.log.debug(`Redis plugin Error setErrorHandler ${error.code}`)
})

fastify.addHook('onError', function (error, request, reply) {
  fastify.log.debug(`Redis plugin Error onError ${error.code}`)
})

fastify.register(require('@fastify/redis'), {
  host: config.redis.hosts,
  password: config.redis.password
});
mcollina commented 1 year ago

You might need to add some additional logging in https://github.com/fastify/fastify-redis/blob/master/index.js#L61-L99.

Currently, this plugin assumes it can connect to the Redis instance. There is no way to "try again" if the connection fails or the time it tasks is over the time allotted.

stefanocudini commented 1 year ago

@mcollina ok now it's clear to me!

I will try to send a PR.. Can you tell me if I can use part of the same code from the mongo plugin? I think that mongo plugin is more evolved than redis

mcollina commented 1 year ago

Sure reuse whatever you find useful. Fastify is mainly driven by community contributions, and different modules attract different level of contributions.

agjs commented 1 year ago

@stefanocudini - I'm having the same issue. Did you figure this one out yet? If you need help, I'm happy to pair up and do a PR on this one.

Eomm commented 1 year ago

Do it solve? https://github.com/fastify/fastify-redis/pull/177

stefanocudini commented 1 year ago

Do it solve? #177

the fix in #177 (adding a new option) solve a different problem: to close redis connetion when fastify app is closed. This issue is about the opposite problem: Redis connection closed that crashes the fastify app.

The main problem, as explained above, is that there isn't a way to catch errors of this type on the fastify side. This is an unsolved problem

Eomm commented 1 year ago

but I just want to catch the error: FST_ERR_PLUGIN_TIMEOUT, how can I do this?

const app = require('fastify')({ logger: true, pluginTimeout: 1000 })

app.register(function plugin (app, opts, next) {
  // don't call next()
}).after(function (err) {
  console.log({ err })// timeout error
})

app.ready()
stefanocudini commented 1 year ago

but I just want to catch the error: FST_ERR_PLUGIN_TIMEOUT, how can I do this?

const app = require('fastify')({ logger: true, pluginTimeout: 1000 })

app.register(function plugin (app, opts, next) {
  // don't call next()
}).after(function (err) {
  console.log({ err })// timeout error
})

app.ready()

unfortunately this doesn't cover the case where the connection to redis drops after plugin is registration..

stefanocudini commented 1 year ago

@stefanocudini - I'm having the same issue. Did you figure this one out yet? If you need help, I'm happy to pair up and do a PR on this one.

@agjs a partial solution for my case is it:

fastify.register(require('@fastify/redis'), {
  host: config.redis.hosts,
  password: config.redis.password,
  maxRetriesPerRequest: 100,
  retryDelayOnFailover: 500,
  closeClient: true, // force to close the redis connection when app stopped
})
.after(err => {
  const {redis} = fastify;
  if (err) {
    fastify.log.error('Redis connection error')
  }
  redis.on('error', function(err) {
    fastify.log.error('Redis connection error')
  })
})

@Eomm thank you very much for your suggestion. I think solved this way with the latest version of the plugin and using some additional parameters

2ico commented 4 months ago

I was trying to fix the same issue but I get this:

    redis.on('error', function (err) {
          ^

TypeError: redis2.on is not a function
2ico commented 4 months ago

Edit: my bad, I should include the namespace

redis.NAMESPACE.on('error',...)