fix(provider-redis): cache anything at all on a Redis Cluster

.github/workflows/tests.yml

@@ -78,6 +78,46 @@ jobs:
           node-version: 24
       - run: node scripts/provider-smoke.cjs
 
+  # A real three-master Redis Cluster. The recording client in test:cluster
+  # proves which commands the provider issues, but it cannot see a failure
+  # inside @keyv/redis: its cross-slot MULTI on every write was refused by a
+  # real cluster and swallowed by keyv, so nothing was ever cached there.
+  # Service containers cannot form a cluster, so the nodes are started here
+  # on the host network. See
+  # https://github.com/strapi-community/plugin-rest-cache/issues/100
+  redis_cluster:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
+      - uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6
+
+      - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7
+        with:
+          node-version: 22
+
+      - name: Start a Redis Cluster
+        run: |
+          for port in 7000 7001 7002; do
+            docker run -d --rm --name "redis-$port" --network host redis:8.10 \
+              redis-server --port "$port" --cluster-enabled yes \
+              --cluster-config-file "nodes-$port.conf" --save "" --appendonly no
+          done
+          for port in 7000 7001 7002; do
+            until docker exec redis-7000 redis-cli -p "$port" ping >/dev/null 2>&1; do sleep 1; done
+          done
+          docker exec redis-7000 redis-cli --cluster create \
+            127.0.0.1:7000 127.0.0.1:7001 127.0.0.1:7002 --cluster-replicas 0 --cluster-yes
+          until docker exec redis-7000 redis-cli -p 7000 cluster info | grep -q 'cluster_state:ok'; do
+            sleep 1
+          done
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      - run: pnpm run build
+
+      - run: pnpm run test:cluster:live
+
   linters:
     runs-on: ubuntu-latest
     strategy:

package.json

@@ -37,7 +37,8 @@
     "test:admin:ui": "playwright test -c e2e/playwright.config.ts --ui",
     "docs:api": "typedoc",
     "docs:build:only": "vitepress build docs",
-    "build:providers": "pnpm --filter \"./packages/provider-*\" run build"
+    "build:providers": "pnpm --filter \"./packages/provider-*\" run build",
+    "test:cluster:live": "node scripts/redis-cluster-live.cjs"
   },
   "license": "MIT",
   "devDependencies": {

packages/provider-rest-cache-redis/src/RedisCacheProvider.ts

@@ -71,6 +71,61 @@ interface KeyvRedisStore {
 const redisStoreOf = (cache: Cache): KeyvRedisStore =>
   cache.stores[0].opts.store as unknown as KeyvRedisStore;
 
+/** @keyv/redis' own read/write methods, which a cluster needs replaced. */
+interface KeyvRedisAdapter extends KeyvRedisStore {
+  redis: RedisClient & {
+    set(key: string, value: string, ...args: unknown[]): Promise<unknown>;
+    sadd(key: string, ...members: string[]): Promise<unknown>;
+  };
+  _getKeyName(key: string): string;
+  set(key: string, value: string | undefined, ttl?: number): Promise<void>;
+  delete(key: string): Promise<boolean>;
+}
+
+/**
+ * Make @keyv/redis' writes and deletes work on a Redis Cluster.
+ *
+ * @keyv/redis 3 writes an entry and adds it to the tracking set in one MULTI,
+ * and deletes them the same way. Those are two keys in different hash slots,
+ * which a cluster refuses ("All keys in the pipeline should belong to the same
+ * slots allocation group"). Keyv catches the failure and only emits it, so on
+ * a cluster set() reported success and nothing was ever cached.
+ *
+ * Here the same two commands are issued separately, leaving the same keys, the
+ * same PX lifetime and the same tracking set. The entry is written before it
+ * is tracked and untracked after it is removed, so an interruption leaves at
+ * worst an untracked entry that expires - never a tracked key with nothing
+ * behind it. Standalone servers keep the MULTI.
+ *
+ * See https://github.com/strapi-community/plugin-rest-cache/issues/100
+ */
+function makeClusterSafe(store: KeyvRedisAdapter): void {
+  const { redis } = store;
+
+  store.set = async (key, value, ttl) => {
+    if (value === undefined) return;
+    const name = store._getKeyName(key);
+    // ttl is milliseconds, as PX expects. Never converted.
+    if (typeof ttl === 'number') {
+      await redis.set(name, value, 'PX', ttl);
+    } else {
+      await redis.set(name, value);
+    }
+    if (store.opts?.useRedisSets !== false) {
+      await redis.sadd(store._getNamespace!(), name);
+    }
+  };
+
+  store.delete = async (key) => {
+    const name = store._getKeyName(key);
+    const removed = Number(await redis.unlink(name));
+    if (store.opts?.useRedisSets !== false) {
+      await redis.srem(store._getNamespace!(), name);
+    }
+    return removed > 0;
+  };
+}
+
 /** Everything accepted under `provider.options` in the plugin configuration. */
 export interface RedisCacheProviderOptions {
   /** Name of a connection declared on the redis plugin. */
@@ -90,14 +145,15 @@ export class RedisCacheProvider extends CacheProvider {
 
     const { ttl, ...adapterOptions } = options;
 
+    const store = new KeyvRedis(client, adapterOptions);
+    if (client.isCluster) {
+      makeClusterSafe(store);
+    }
+
     this.client = client;
     this.cache = createCache({
       ttl,
-      stores: [
-        new Keyv({
-          store: new KeyvRedis(client, adapterOptions),
-        }),
-      ],
+      stores: [new Keyv({ store })],
     });
   }
 

scripts/redis-cluster-live.cjs

@@ -0,0 +1,133 @@
+#!/usr/bin/env node
+'use strict';
+
+/**
+ * Drives the redis provider against a real Redis Cluster.
+ *
+ * redis-cluster-check.cjs proves which commands the provider issues, using a
+ * recording client. That cannot see a failure inside a dependency: @keyv/redis
+ * writes an entry and its tracking-set member in one MULTI, two keys in
+ * different hash slots, which a cluster refuses with
+ *
+ *   All keys in the pipeline should belong to the same slots allocation group
+ *
+ * Keyv catches that and emits it as an event, so set() reported success and
+ * nothing was ever cached on a cluster. Only a real cluster shows it.
+ *
+ * Needs a cluster to write to: REDIS_CLUSTER_NODES, comma-separated host:port
+ * (default 127.0.0.1:7000,127.0.0.1:7001,127.0.0.1:7002).
+ *
+ * @see https://github.com/strapi-community/plugin-rest-cache/issues/100
+ */
+
+const assert = require('assert');
+const path = require('path');
+const { createRequire } = require('module');
+
+const ROOT = path.join(__dirname, '..');
+const PROVIDER = path.join(ROOT, 'packages/provider-rest-cache-redis');
+const requireFromProvider = createRequire(path.join(PROVIDER, 'package.json'));
+// The ioredis the provider actually runs on is @keyv/redis's.
+const Redis = createRequire(requireFromProvider.resolve('@keyv/redis'))('ioredis');
+const { RedisCacheProvider } = require(path.join(PROVIDER, 'dist/RedisCacheProvider.js'));
+
+const NODES = (process.env.REDIS_CLUSTER_NODES || '127.0.0.1:7000,127.0.0.1:7001,127.0.0.1:7002')
+  .split(',')
+  .map((node) => {
+    const [host, port] = node.split(':');
+    return { host, port: Number(port) };
+  });
+
+// Spread across hash slots, as request paths are.
+const KEYS = Array.from({ length: 30 }, (_, i) => `/api/articles?page=${i}&`);
+const MAX_AGE = 600000;
+
+const results = [];
+async function check(name, fn) {
+  try {
+    await fn();
+    results.push([true, name, '']);
+  } catch (error) {
+    results.push([false, name, error.message.split('\n')[0]]);
+  }
+}
+
+async function flushAll(cluster) {
+  await Promise.all(cluster.nodes('master').map((node) => node.flushdb()));
+}
+
+/** Every key on every master. */
+async function allKeys(cluster) {
+  const per = await Promise.all(cluster.nodes('master').map((node) => node.keys('*')));
+  return per.flat().sort();
+}
+
+(async () => {
+  const cluster = new Redis.Cluster(NODES);
+  await new Promise((resolve, reject) => {
+    cluster.once('ready', resolve);
+    cluster.once('error', reject);
+  });
+
+  const provider = new RedisCacheProvider(cluster, {});
+  // Keyv reports store failures as events, not rejections. Collect them, so a
+  // swallowed CROSSSLOT fails a check instead of passing it.
+  const storeErrors = [];
+  provider.cache.stores[0].on('error', (error) => storeErrors.push(error.message));
+  const noStoreErrors = () =>
+    assert.deepStrictEqual(storeErrors.splice(0), [], 'the store reported an error');
+
+  await check('set() stores entries that get() returns', async () => {
+    await flushAll(cluster);
+    for (const key of KEYS) await provider.set(key, { key }, MAX_AGE);
+    noStoreErrors();
+    for (const key of KEYS) assert.deepStrictEqual(await provider.get(key), { key }, key);
+  });
+
+  await check('set() tracks every entry, with its lifetime', async () => {
+    const tracked = (await cluster.smembers('namespace:keyv')).sort();
+    assert.deepStrictEqual(tracked, KEYS.map((k) => `keyv:${k}`).sort());
+    const ttl = await cluster.pttl(`keyv:${KEYS[0]}`);
+    assert.ok(ttl > MAX_AGE - 10000 && ttl <= MAX_AGE, `lifetime ${ttl}ms is not maxAge`);
+  });
+
+  await check('keys() lists every entry', async () => {
+    assert.deepStrictEqual((await provider.keys()).sort(), [...KEYS].sort());
+  });
+
+  await check('del() removes the entry and its tracking', async () => {
+    await provider.del(KEYS[0]);
+    noStoreErrors();
+    assert.ok((await provider.get(KEYS[0])) == null, 'entry survived del()');
+    assert.strictEqual(await cluster.sismember('namespace:keyv', `keyv:${KEYS[0]}`), 0);
+  });
+
+  await check('delMany() removes entries across slots', async () => {
+    await provider.delMany(KEYS.slice(1, 15));
+    noStoreErrors();
+    for (const key of KEYS.slice(1, 15)) {
+      assert.ok((await provider.get(key)) == null, `${key} survived delMany()`);
+    }
+    assert.deepStrictEqual((await provider.keys()).sort(), KEYS.slice(15).sort());
+  });
+
+  await check('clear() empties the cluster', async () => {
+    await provider.clear();
+    noStoreErrors();
+    assert.deepStrictEqual(await allKeys(cluster), []);
+  });
+
+  await flushAll(cluster);
+  cluster.disconnect();
+
+  console.log(`redis cluster (live) - ${NODES.length} nodes\n`);
+  for (const [ok, name, detail] of results) {
+    console.log(`  ${ok ? 'PASS' : 'FAIL'}  ${name}${detail ? `\n          ${detail}` : ''}`);
+  }
+  const failed = results.filter(([ok]) => !ok).length;
+  console.log(`\n${results.length - failed}/${results.length} passed`);
+  process.exit(failed > 0 ? 1 : 0);
+})().catch((error) => {
+  console.error(error);
+  process.exit(1);
+});