fix(redis): detect an existing connection by shape instead of instanceof

[DavideCarvalho]

Fixes #122.

The bug

RedisDriver and redisBusDriver decide whether connection is a live client or a plain options object with instanceof:

if (config.connection instanceof IoRedis || config.connection instanceof IoRedisCluster) {
  this.#connection = config.connection
  return
}

this.#connection = new IoRedis(config.connection)   // <- silent fallback

instanceof is evaluated against the ioredis copy bentocache resolved. When the host application resolves a different ioredis major (two copies in the tree, which is the normal pnpm outcome), the check is false for a perfectly valid client, and we fall through to new IoRedis(<a live Redis instance treated as an options bag>). ioredis ignores the unrecognised properties and connects to 127.0.0.1:6379 — silently, with no error.

I reproduced the end-to-end failure with the built package, an app on ioredis@6.0.0 and bentocache resolving its own ioredis@5.11.1, against a Redis on a non-default port:

	extra connections opened	where the value landed
main	1 (to 127.0.0.1:6379)	not on the configured server
this PR	0	the configured server

Note that bento.get() returned the right value in both runs, because L1 served it. That is why this is so hard to spot: everything looks fine until L1 misses, and then L2 is empty and a second connection is quietly hammering localhost.

Real-world impact

• Redis driver instanceof check fails across ioredis majors and silently falls back to 127.0.0.1:6379 #122 — the report this PR closes.
• v10.0.1 bumps ioredis to v6 in a patch release; breaks @adonisjs/cache (bentocache instanceof check) and connects to 127.0.0.1 adonisjs/redis#77 — @adonisjs/redis@10.0.1 shipped ioredis 6 in a patch, which broke @adonisjs/cache (it passes ioConnection straight to bentocache) and spammed connect ECONNREFUSED 127.0.0.1:6379 every ~2s in production. It was resolved by reverting to ioredis 5 and re-shipping ioredis 6 as the 11.0.0 major — so today @adonisjs/cache still cannot move to @adonisjs/redis@11, and this check is the reason.
• ioredis 版本冲突 adonisjs/cache#18 — same version conflict, reported downstream.

What changed

Both call sites now go through one duck-type helper:

function isIoRedisClient(connection: unknown): connection is IoRedis | IoRedisCluster {
  if (typeof connection !== 'object' || connection === null) return false

  const candidate = connection as Partial<IoRedis>
  return typeof candidate.duplicate === 'function' && typeof candidate.sendCommand === 'function'
}

Why these two properties:

• They are on the prototype of both Redis and Cluster, in every ioredis major. I verified this against 5.11.1 and 6.0.0 for all four combinations (v5.Redis, v5.Cluster, v6.Redis, v6.Cluster).
• Neither name exists in RedisOptions or ClusterOptions in either major (I grepped the shipped .d.ts files), so a legitimate options object can never be misclassified as a client. This matters more than the reverse: a false positive fails loudly on the first command, a false negative is the silent-localhost bug we are fixing.
• constructor.name is not usable here, in case it comes up in review — ioredis builds its clients through a mixin, so both Redis and Cluster report EventEmitter in both majors.

On throwing vs. the silent fallback

Issue #122 asks for a loud failure. I kept the existing fallback for genuine options objects — that is the documented API and changing it would be breaking — but added a narrow tripwire: if the value is not recognised as a client yet still carries the markers of one (a status string, an options object and emit), we throw InvalidArgumentsException instead of dialing localhost. None of those three names are valid option names either, so no legitimate config trips it.

This branch is unreachable for every ioredis major that exists today — it only exists so that a future shape change degrades into a boot-time error rather than another production incident. Happy to drop that hunk if you would rather keep the diff to just the discriminator.

Peer range: I did not widen it, and here is why

I validated ioredis 6 properly before deciding, since #122 also suggests widening to ^5.3.2 || ^6.0.0.

RESP3 is really negotiated (options.protocol === 3, and the server's HELLO agrees), and the whole RedisDriver surface is clean on it — against Redis 7.4:

get (hit + miss), getdel (hit + miss), set, set … 'PX' ttl, scan with MATCH/COUNT returning [cursor, keys] with the keyPrefix still on the keys, pipeline() + unlink + exec() returning [[err, res], …], bare unlink, and connection.options.keyPrefix. All identical to ioredis 5.

The bus path also holds up. I ran a real binary round-trip through Redis: a 363-byte BinaryEncoder payload (NUL bytes, a multi-byte UTF-8 key, a 300-char key) published on one duplicated client and received on another via messageBuffer, exactly as RedisTransport does with useMessageBuffer: true. Under ioredis 6 / RESP3 the received Buffer is byte-identical to what was published and BinaryEncoder.decode round-trips exactly. (For contrast, the non-buffer message string path is lossy in both majors — no regression there, just a reminder of why useMessageBuffer is set.)

So the surface itself is fine on 6. The blocker is elsewhere: @boringnode/bus — a hard dependency of bentocache — peers ioredis ^5.0.0 (still true at 0.9.2) and its RedisTransport repeats the exact same instanceof mistake:

if (options instanceof Redis || options instanceof Cluster) { /* duplicate() */ }
this.#publisher = new Redis(options)   // same silent fallback, one layer down

I confirmed this is live, not theoretical: with this PR applied, a foreign-major client handed to redisBusDriver is now correctly forwarded to RedisTransport, which then fails its instanceof and would open publisher/subscriber connections to 127.0.0.1:6379 with useMessageBuffer silently falling back to false (a binary payload decoded from a lossy string — cf. #19).

Widening bentocache's peer to ^6.0.0 while a transitive dependency still peers ^5.0.0 would produce unmet-peer warnings for every ioredis-6 user and hard-fail strict installers, without actually making the bus work across majors. That felt like your call rather than mine, so this PR leaves ioredis at ^5.3.2 and is purely the duck-typing fix. If you want, I am happy to send the same patch to @boringnode/bus and then follow up here with the peer bump — just say the word.

Scoping it this way also means the fix is a strict improvement on its own: the L2 cache path, which is what actually broke in adonisjs/redis#77, is fully fixed by this PR alone.

Tests

In tests/drivers/redis.spec.ts. The cross-major cases now use a real second ioredis major, installed side by side through an npm alias:

"devDependencies": {
  "ioredis-v6": "npm:ioredis@^6.0.0"
}

so the driver is handed a genuine ioredis@6.0.0 client while it keeps resolving its own ioredis@5 — no shape-faithful stand-in anymore. Each of those cases asserts the fixture really is a different class before using it, assert.instanceOf(client, IoRedisV6) + assert.notInstanceOf(client, IoRedis), the second one being the exact condition that used to send us down the wrong branch.

• a real ioredis 6 Redis is reused, and the write lands on the connection it was handed: that client is pointed at db 3, the value written through the driver is read back there, and a second client on the db 0 the silent fallback targets sees nothing
• a real ioredis 6 Cluster is reused
• a plain options object still builds a new connection, with host / port / keyPrefix carried over
• an unrecognisable client-shaped object throws instead of falling back to localhost
• redisBusDriver forwards a foreign-major client to RedisTransport instead of shallow-copying it into an options bag
• redisBusDriver throws on an unrecognisable client-shaped object

Five of the six fail on main and pass here. The sixth is the options-object one — it guards the other branch against a regression, so it passes on both; I would rather say that than round it up to six.

Three of them keep a hand-made object on purpose, because a real client cannot express what they cover:

• the two "unrecognisable client" tests — something shaped like a client that is not any ioredis is by definition not something an install can produce. That is the tripwire branch, and it is the only case the old Proxy fixture covered that a real client does not.
• the bus forwarding test, whose recorder now fronts a real v6 client. RedisTransport keeps its connection private, and a bare v6 client cannot tell the two code paths apart from the outside: the transport repeats the same instanceof check and builds its own connection either way (cf. the peer-range section above and fix(redis): detect an existing connection by shape instead of instanceof, and support ioredis 6 boringnode/bus#71). So the recorder observes the only thing that is ours to get right here — that the connection is passed by reference. A null-prototype target has no own enumerable key, so { ...connection } would read nothing, and any property access proves the object itself was forwarded.

What I ran

• Redis driver 30 passed (29 passed / 1 skipped with CI=1, the real-cluster test). The Redis + Valkey + File + Memory + DynamoDB driver specs together: 130 passed, 0 failed — byte-for-byte the same counts before and after this test change.
• Full unit suite (which is where the bus specs live): 244 passed, 0 failed. The Tagging | deleteByTag / can remove by tag timing flake I hit on main earlier did not reproduce in this run.
• Reverting src/drivers/redis.ts to the instanceof version with the new tests in place: 25 passed, 5 failed of the file's 30 — the five listed above, failing on getConnection() === client, on the two tripwires not throwing, and on the bus connection not being forwarded. That run also never exits on its own: the stray localhost client the old code builds is never closed, which is the leak side of the same bug.
• pnpm lint and pnpm typecheck clean.

What I did not validate

• The Postgres/MySQL driver suites — my machine's 5432 was occupied, and they are untouched by this change.
• Everything above is Redis 7.4 only: no Valkey, no real cluster and no sentinel under ioredis 6. The cross-major Cluster case does build a real ioredis 6 Cluster, but with lazyConnect, so it covers the detection rather than a live cluster round-trip.

[changeset-bot]

🦋 Changeset detected

Latest commit: f12c397

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
bentocache	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[DavideCarvalho]

Follow-up on the peer-range section above: I sent the same patch to @boringnode/bus — boringnode/bus#71.

That is the layer that was blocking this one. It fixes the identical instanceof check in RedisTransport with the same duplicate + sendCommand discriminator, and — because nothing sits below it — it also widens the bus peer to ioredis: "^5.0.0 || ^6.0.0" after validating the full transport surface on ioredis 6 / RESP3 (options.protocol === 3 confirmed): pub/sub, unsubscribe, the messageBuffer binary path byte-for-byte, reconnection, and a real three-node cluster. Full existing bus suite green under both majors.

It also covers the second symptom I mentioned here, which is specific to the bus: on the unrecognised-client path useMessageBuffer silently degraded to false, so binary payloads were decoded from a lossy string. Two-major repro, against the built package with the app on ioredis 6 and the bus on ioredis 5:

	duplicate() calls	sockets to :6379	reached the configured server	useMessageBuffer honoured	payload decoded
bus main	0	5	no	no	no
boringnode/bus#71	4	0	yes	yes	yes

So once that lands and ships, the peer bump here becomes safe and I am happy to follow up with it — which is the last thing standing between @adonisjs/cache and @adonisjs/redis@11 (adonisjs/cache#18).