Skip to content

fixed ttl inconsistency between usage of ioredis and node-redis clients#28

Open
SerMedvid wants to merge 2 commits intomrjasonroy:mainfrom
SerMedvid:fix/redis-client-inconsistency
Open

fixed ttl inconsistency between usage of ioredis and node-redis clients#28
SerMedvid wants to merge 2 commits intomrjasonroy:mainfrom
SerMedvid:fix/redis-client-inconsistency

Conversation

@SerMedvid
Copy link

@SerMedvid SerMedvid commented Feb 22, 2026

Description

The data cache Redis implementation is designed to use a node-redis instance (as indicated by the examples and Redis interface). However, the TTL is currently being set using ioredis syntax, which is incompatible with node-redis. As a result, the TTL on redis entry is not being set at all.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Chore (dependencies, CI, etc.)

Testing

  • All existing tests pass (pnpm test)
  • Added new tests for new functionality
  • Tested with memory cache handler
  • Tested with Redis cache handler (if applicable)
  • E2E tests pass (pnpm test:e2e)

Checklist

  • My code follows the project's code style (pnpm lint passes)
  • I have run pnpm format to format my code
  • Type checking passes (pnpm typecheck)
  • I have updated the documentation accordingly
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

Additional Context

@gemini-code-assist
Copy link

Summary of Changes

Hello @SerMedvid, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an inconsistency in how Time-To-Live (TTL) is applied to Redis cache entries. The previous implementation used ioredis syntax for setting TTL, which was incompatible with the intended node-redis client, resulting in TTLs not being set. The changes update the Redis SET command arguments to align with the node-redis client's expected format, ensuring cache entries expire as intended and improving the reliability of the caching mechanism.

Highlights

  • Redis TTL Format: The redis.set command's Time-To-Live (TTL) argument format was changed from ioredis style ("EX", seconds) to node-redis style ({ EX: seconds }) to ensure proper expiration of cache entries.
  • Redis Client Compatibility: The RedisClient interface and associated comments were updated to explicitly state compatibility with node-redis instead of ioredis, clarifying the intended client.
  • Test Suite Alignment: The FakeRedis mock implementation and test assertions were updated to correctly interpret and verify the node-redis style TTL options, ensuring the fix is thoroughly tested.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/cache-handler/src/data-cache/redis.test.ts
    • Updated the FakeRedis class to correctly parse node-redis style SET options for TTL.
    • Modified test assertions to expect node-redis style SET arguments for TTL.
    • Renamed a test description to reflect the use of node-redis style options.
  • packages/cache-handler/src/data-cache/redis.ts
    • Updated comments in RedisDataCacheHandlerOptions and RedisClient interface to specify node-redis compatibility.
    • Modified the redis.set call to use an object { EX: Math.ceil(ttl) } for setting TTL, aligning with node-redis client's API.
Activity
  • The author classified this change as a bug fix.
  • The author confirmed that all existing tests pass.
  • The author confirmed testing with the Redis cache handler.
  • The author confirmed that code style, type checking, and unit tests pass locally.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request successfully fixes a bug where the TTL for Redis cache entries was not being set correctly. The change from ioredis-style arguments to node-redis object options for the set command is correct and addresses the issue. While the code changes are good, I've noted that related documentation and examples in the repository have not been updated to reflect the switch from ioredis to node-redis. An update is needed to maintain consistency and avoid confusion.

export interface RedisDataCacheHandlerOptions {
/**
* Redis client instance (ioredis compatible)
* Redis client instance (node-redis)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This change correctly specifies that a node-redis client instance is expected. However, other parts of the repository still refer to ioredis. To ensure consistency and prevent confusion for users of this package, it's important to update all related documentation and examples to reflect this switch.

Specifically:

  • docs/redis.md still contains instructions and examples for ioredis.
  • apps/redis-example/data-cache-handler.mjs also uses ioredis for its client setup.

These should be updated to use and refer to node-redis.

Copy link
Owner

@mrjasonroy mrjasonroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying this inconsistency, @SerMedvid! The intent is correct — the RedisClient interface and docs target node-redis, so the set() call should use node-redis-style { EX: seconds }.

However, this change will silently break TTL when used through createCacheHandler() (the recommended/primary path for most users).

The issue is in packages/cache-handler/src/data-cache/factory.ts — the ioredis adapter only handles the old calling convention:

// factory.ts:32-37
set: (key, value, exFlag?, ttl?) => {
  if (exFlag === "EX" && typeof ttl === "number") {
    return redis.set(key, value, "EX", ttl);
  }
  return redis.set(key, value); // <-- TTL silently dropped!
},

After this PR, the call becomes redis.set(key, value, { EX: seconds }). The adapter receives { EX: 120 } as exFlag, the === "EX" check fails, and it falls through to redis.set(key, value)TTL is silently discarded.

To make this work, the factory adapter needs to be updated to handle the node-redis-style object arg:

set: (key, value, ...args) => {
  const opts = args[0];
  if (opts && typeof opts === "object" && typeof (opts as Record<string, unknown>).EX === "number") {
    return redis.set(key, value, "EX", (opts as Record<string, unknown>).EX as number);
  }
  if (args[0] === "EX" && typeof args[1] === "number") {
    return redis.set(key, value, "EX", args[1]);
  }
  return redis.set(key, value);
},

Plus a test covering the factory path with TTL.

Would you be open to updating the PR to include the factory adapter change? Happy to help if needed.

SerMedvid and others added 2 commits February 24, 2026 11:19
The RedisClient interface now uses node-redis style SET options
({ EX: seconds }). The factory adapter must translate these to
ioredis's positional format ("EX", seconds).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mrjasonroy mrjasonroy force-pushed the fix/redis-client-inconsistency branch from 33c09f6 to ad841ce Compare February 24, 2026 19:20
@mrjasonroy
Copy link
Owner

Rebased onto main and added a commit to update the factory adapter (createRedisAdapter()) to translate node-redis style { EX: seconds } to ioredis positional args. Without this, TTL was being silently dropped for users going through createCacheHandler().

All 95 tests pass. Ready to merge once CI is green.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants