cache-manager icon indicating copy to clipboard operation
cache-manager copied to clipboard

Implement cache client event listeners

Open aaroncraigongithub opened this issue 2 years ago • 3 comments
trafficstars

Is there an existing issue that is already proposing this?

  • [X] I have searched the existing issues

Is your feature request related to a problem? Please describe it

I often use console commands for long running jobs such as data migrations. I also have some workers that run in a headless Nest application.

In both instances, I occasionally have issues where the Redis cache client throws an ETIMEDOUT error. This has the effect of crashing the entire program, and no amount of try/catching seems to catch the error.

I believe the correct way to deal with this is to set up an event listener on the redis client itself, however I have not been able to find a way to get the client once it's been instantiated.

Unless I'm missing something, there is no opportunity to set up an error listener in the following example code:

    CacheModule.registerAsync<RedisClientOptions>({
      useFactory: async () => {
        const config = await getRedisConfig();

        return {
          ...config,
          store: redisStore,
        };
      },
    }),

Describe the solution you'd like

Being able to grab the client and set up listeners somewhere before it is used seems to be important. Ideally one could set up listeners at various touchpoints:

  • in the configuration itself
  • by exposing the client (or an .on() method) at runtime

Event listeners may be specific to the Redis client, in which case this would be a better request for that library.

Teachability, documentation, adoption, migration strategy

A possible API

Setting up listeners at config time:

    CacheModule.registerAsync<RedisClientOptions>({
      useFactory: async () => {
        const config = await getRedisConfig();

        return {
          ...config,
          store: redisStore,
          on: {
            error: globalCacheErrorHandler,
            // other event listeners
          }
        };
      },
    }),

Setting up listeners at run time

 @Injectable()
export class CacheService {
  constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {
    this.cacheManager.on('error', this.handleCacheError);
  }

  private handleCacheError(err: Error) {
    // ....
  }
}

What is the motivation / use case for changing the behavior?

As it stands, it's impossible to react to events emitted by the underlying client.

aaroncraigongithub avatar Feb 16 '23 05:02 aaroncraigongithub

For listening to the events from RedisClient, once the module has been initialized, I could write a listener for it as follows (I am using cache-manager-redis-store):

export class RedisClientEventListener implements OnModuleInit, OnModuleDestroy {
  private readonly logger = new Logger(RedisClientEventListener.name);
  private readonly redisStore: RedisStore;

  constructor(@Inject(CACHE_MANAGER) readonly cacheManager: Cache) {
    this.redisStore = cacheManager.store as unknown as RedisStore;
  }

  onModuleInit(): void {
    const client = this.redisStore.getClient();

    if (!client) {
      this.logger.error('no redis client initialised');
    }

    if (client.isReady) {
      this.logger.log('redis client is connected and ready');
    }

    client.on('connect', () => {
      this.logger.log('redis client is connecting to server...');
    });

    client.on('ready', () => {
      this.logger.log('redis client is ready');
    });

    client.on('end', () => {
      this.logger.log('redis client connection closed');
    });

    client.on('error', (error: Error) => {
      this.logger.error(error, 'redis client received an error');
    });
  }

  async onModuleDestroy(): Promise<void> {
    await this.redisStore.getClient().quit();
  }
}

and then the module that imports and initializes CacheModule extends the above:

@Module({
  imports: [
    CacheModule.registerAsync(cacheModuleOptions),
  ],
  controllers: [SomeController],
})
export class SomeModule extends RedisClientEventListener {}

This works well to listen to the errors and handle those and the app doesn't crash, however, if the app is unable to connect to the client during initialisation itself, these listeners don't get a change to get registered and the app crashes. So far even I haven't found a way to avoid the app from crashing when app is unable to connect while initialising CacheModule.

nishkarsh-beamery avatar Feb 17 '23 20:02 nishkarsh-beamery

anything new on this issue?

Mut1aq avatar Jun 17 '23 22:06 Mut1aq

For listening to the events from RedisClient, once the module has been initialized, I could write a listener for it as follows (I am using cache-manager-redis-store):

export class RedisClientEventListener implements OnModuleInit, OnModuleDestroy {
  private readonly logger = new Logger(RedisClientEventListener.name);
  private readonly redisStore: RedisStore;

  constructor(@Inject(CACHE_MANAGER) readonly cacheManager: Cache) {
    this.redisStore = cacheManager.store as unknown as RedisStore;
  }

  onModuleInit(): void {
    const client = this.redisStore.getClient();

    if (!client) {
      this.logger.error('no redis client initialised');
    }

    if (client.isReady) {
      this.logger.log('redis client is connected and ready');
    }

    client.on('connect', () => {
      this.logger.log('redis client is connecting to server...');
    });

    client.on('ready', () => {
      this.logger.log('redis client is ready');
    });

    client.on('end', () => {
      this.logger.log('redis client connection closed');
    });

    client.on('error', (error: Error) => {
      this.logger.error(error, 'redis client received an error');
    });
  }

  async onModuleDestroy(): Promise<void> {
    await this.redisStore.getClient().quit();
  }
}

and then the module that imports and initializes CacheModule extends the above:

@Module({
  imports: [
    CacheModule.registerAsync(cacheModuleOptions),
  ],
  controllers: [SomeController],
})
export class SomeModule extends RedisClientEventListener {}

This works well to listen to the errors and handle those and the app doesn't crash, however, if the app is unable to connect to the client during initialisation itself, these listeners don't get a change to get registered and the app crashes. So far even I haven't found a way to avoid the app from crashing when app is unable to connect while initialising CacheModule.

I am stuck on the exactly the same thing :(

diskra avatar Feb 28 '24 16:02 diskra

There's not much we can control here, as we're relying on the behavior of the cache-manager. However, I was able to put together an example that demonstrates how you could listen for initialization errors, and, if Redis is unavailable, fall back to an in-memory store:

CacheModule.registerAsync({
  useFactory: async () => {
    try {
      const store = await redisStore({
        socket: {
          host: 'localhost',
          port: 6380,
        },
      });

      // Listen for Redis connection events
      store.client.on('connect', () => {
        console.log('Connected to Redis');
      });

      store.client.on('error', err => {
        console.log('Redis error:', err);
      });

      // If we reach this point, Redis is available
      return [
        {
          store: 'memory',
          max: 100,
          ttl: 50,
        },
        {
          store: store as unknown as CacheStore,
          ttl: 3 * 60000, // 3 minutes (milliseconds)
        },
      ];
    } catch (err) {
      console.error('Error initializing Redis:', err);

      // Fallback to in-memory store
      return [
        {
          store: 'memory',
          max: 100,
          ttl: 50,
        },
      ];
    }
  },
}),

redisStore is imported from cache-manager-redis-yet

In the next major release, we're going to upgrade to v6 (ref https://github.com/nestjs/cache-manager/pull/508) - this should make things a bit easier moving forward

kamilmysliwiec avatar Oct 21 '24 08:10 kamilmysliwiec