Question about reconnection and expired credentials

[fturmel]

Hi! Hoping to test your cache handler on a project that is deployed on Azure services (AKS, Azure Cache for Redis).

We are looking into using a token (with TTL) as the password for the Redis resource, and I am wondering what behavior can be expected if the handler gets disconnected in this scenario. The token used during the initial connection may very well be expired at that point and the reconnection would fail. Based on your docs, it doesn't seem like there would be a way to get a fresh token when we try to reconnect, right?

If you want to see what this looks like in the scenario where you manage the Redis client directly, here's an example from the Azure SDK docs:
https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/identity/identity/samples/AzureCacheForRedis/node-redis.md

Thoughts? Thanks in advance!

[tilman]

Hey, you are correct. We have not thought about such a case and therefore constructing the redis client in our cache handler class by our own.

A potential solution could be to extend the RedisStringsHandler options with a redisClient optional field, so you can:

• create a custom cache handler: https://github.com/trieb-work/nextjs-turbo-redis-cache?tab=readme-ov-file#option-b-create-a-wrapper-file-to-change-options
• create your own client (inside this custom handler)
• register error/disconnect event handlers for token revalidation
• pass in the client into RedisStringsHandler

As a workaround you could also use KILL_CONTAINER_ON_ERROR_THRESHOLD env var. If the connection is lost and reconnecting is tried with an invalid token, the process will be killed after an defined amount of failed connection attempts. But make sure to implement a container restart procedure/watchdog in your cloud hosting before using this.

If you want, feel free to create a PR with the redisClient change. Make sure that old options are still working, so if no redisClient is provided, it will use the current logic. Also please extend the README with these changes and add a note to the current redis options that they are not used if redisClient is provided.

[fturmel]

Thanks @tilman! I'll follow up in a few days and see if I can contribute a PR to the project.

Some thoughts just browsing through the source code quickly:

• We might also need an option to disable the default reconnect and kill container(already there) behavior inside RedisStringsHandler
• Shouldn't killContainerOnErrorCount reset back to 0 after a successful reconnect?

[fturmel]

Quick update, I am also evaluating other options at the moment and not actively working on a PR just yet.

Managing/creating the Redis client from outside the cache handler would help a lot. The v4.x Redis client requirement however would be blocking us from using @redis/entraid to handle the managed identity/token, which I would highly prefer. Was there any progress made on #31, is this a long way to ever get implemented?

Also it seems #42 echos that the internal reconnect behavior should either be optional or removed.

[tilman]

Thanks @tilman! I'll follow up in a few days and see if I can contribute a PR to the project.

Some thoughts just browsing through the source code quickly:

• We might also need an option to disable the default reconnect and kill container(already there) behavior inside RedisStringsHandler
• Shouldn't killContainerOnErrorCount reset back to 0 after a successful reconnect?

Jep, as you probably already seen (updated text) the kill container thingy is fully optional and is disabled by default. It should be used with extreme care and only been activated if you are sure that your orchestrating services can handle process termination.

Regarding killContainerOnErrorCount reset to 0: my intention of this feature was to restart the container if there are occasionally problems with redis (does not have to be re-connection related). In our case in google cloud runtime we had problems that the hostname was sometimes not resolving correctly. After a restart of the container it was working again. For this problem I have added this feature, for self healing. But as already mentioned I think this is more kind of a edge case and can be ignored by most other users (therefore it is also disabled by default).

[tilman]

Quick update, I am also evaluating other options at the moment and not actively working on a PR just yet.

Managing/creating the Redis client from outside the cache handler would help a lot. The v4.x Redis client requirement however would be blocking us from using @redis/entraid to handle the managed identity/token, which I would highly prefer. Was there any progress made on #31, is this a long way to ever get implemented?

Also it seems #42 echos that the internal reconnect behavior should either be optional or removed.

I don't think #31 should be a major issue — it should just require a search and replace to update the timeout format for all Redis operations using timeouts (as far as I remember, this only affects the get operation).

The more significant task will be to properly test that everything continues to work as expected. Unfortunately, I don't have the time for that at the moment.

However, feel free to proceed with the upgrade and client connection refactoring in a single PR. This way, you can start using the alternative Redis client whenever it suits you and you've decided to adopt this package.

Once I've wrapped up some of my current projects, I’ll have more time to continue developing this package.