Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to log when an error occurs #205

Closed
janmeier opened this issue Apr 18, 2023 · 5 comments
Closed

Make it possible to log when an error occurs #205

janmeier opened this issue Apr 18, 2023 · 5 comments

Comments

@janmeier
Copy link

We are creating a redis rate limiter, with an in-memory insurance limiter. We would like to be notified whenever a call to redis fails, so we can do something about it :)

The way I've done it for now is to subclass RateLimiterRedis:

class LenusRateLimiterRedis extends RateLimiterRedis {
  _handleError(err, funcName, resolve, reject, key) {
    errorTracker.capture('Error in redis rate limiter', { key, funcName }, err)
    super._handleError(err, funcName, resolve, reject, key)
  }
}

This works fine, but since _handleError is not part of the ts interface definition we won't be notified if the definition of _handleError changes.

My idea would be to either:

  • Make _handleError (or a similar method) part of the public interface, so we can subclass is properly
  • Add an onError callback to options
  • Emit an error event

Whichever option you like most, I would be happy to implement it :)

@animir
Copy link
Owner

animir commented Apr 18, 2023

@janmeier Hi, this is an interesting case.
While I understand your requirement, there is another way of thinking about it. We should expect Redis isn't able to handle a request in only one case—a connection error.
You could use redisClient.on('error', (err) => {}) event to catch and log that error.
If you see some issues with the suggested approach, please describe your case in more detail so that we can discuss it.

@janmeier
Copy link
Author

You could use redisClient.on('error', (err) => {}) event to catch and log that error.

That was my initial though, but it seems the connection error is not being emitted (at least not in ioredis). Related issues redis/ioredis#969 and redis/ioredis#1386

While I agree in principle that we could just handle this on the redis level, there have been no maintainer responses to those issues as far as I can see :/

@animir
Copy link
Owner

animir commented Apr 20, 2023

@janmeier I understand your pain. Have you considered using node-redis package? Another option would be to PING Redis server once in a while and log error.

@animir
Copy link
Owner

animir commented Apr 27, 2023

@janmeier Hi, have you found a suitable solution for this?

@animir animir closed this as completed May 4, 2023
@shameersheik
Copy link

shameersheik commented May 30, 2023

Some of the errors didnot trigger redisClient.on('error', (err) => {}) in ioredis but triggered

reconnectOnError: function (err) {
          logger.info(`Got an error when redis tried to connect: ${err.stack}`);
          return true;
        },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants