feat: improve redis cluster handling

[matthewwillian]

Summary

• add queue cache miss logging and allow connections to pass all redis options while preserving retry strategy
• merge redis cluster credentials/tls from node urls, env overrides, and nested redisOptions; add helper for TLS composition
• install dependencies so tsc is available locally and update both lockfiles

Testing

• npm run build

[Copilot]

Pull Request Overview

This PR refactors the Redis Cluster TLS configuration logic in getRedisClient to improve flexibility and maintainability. The changes introduce a more structured approach to merging TLS configurations from multiple sources (environment variables, supplied options, and cluster-level options).

• Extracts TLS environment variable handling into a dedicated function getClusterTlsFromEnv()
• Implements a generic mergeTlsConfigs() function to merge multiple TLS configuration sources with proper precedence
• Refactors cluster Redis options handling to support nested redisOptions with proper username, password, and TLS configuration merging

Comments suppressed due to low confidence (1)

lib/queue-factory.ts:174

• The cache key computation using JSON.stringify(redisOpts) may not include clusterNodes which affects the actual Redis client configuration. When the same redisOpts is used with different clusterNodes, it will incorrectly return the same cached client. The cache key should incorporate clusterNodes to ensure unique clients are created for different cluster configurations.

  // Compute checksum for redisOpts
  const checksumJson = JSON.stringify(redisOpts);
  const checksum = require("crypto")
    .createHash("md5")
    .update(checksumJson)
    .digest("hex");

  const key = `${type}-${checksum}`;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The nullish coalescing operator is unnecessary here since the condition on line 344 already ensures process.env.REDIS_CLUSTER_TLS is truthy. This can be simplified to just process.env.REDIS_CLUSTER_TLS.

Suggested change

	process.env.REDIS_CLUSTER_TLS ?? "",
	process.env.REDIS_CLUSTER_TLS,

[Copilot]

Using delete on properties can impact performance in V8 optimization. Consider using object destructuring to exclude these properties instead: const { username, password, tls, ...baseRedisOptions } = suppliedRedisOptions;

Suggested change

	const baseRedisOptions: NestedRedisOptions = {
	...suppliedRedisOptions,
	};
	delete baseRedisOptions.username;
	delete baseRedisOptions.password;
	delete baseRedisOptions.tls;
	const { username, password, tls, ...baseRedisOptions } = suppliedRedisOptions;

[Copilot]

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] Using Object.assign with an empty object is less idiomatic than using object spread syntax. Consider replacing this with: const mergedTls = { ...tlsFromEnv, ...userRedisOptions.tls, ...redisOpts.tls } as TlsConnectionOptions; Note that you'll need to handle undefined values explicitly or use ?? {} on each source.

Suggested change

	const mergedTls = Object.assign(
	{},
	tlsFromEnv ?? {},
	(userRedisOptions.tls as TlsConnectionOptions | undefined) ?? {},
	(redisOpts.tls as TlsConnectionOptions | undefined) ?? {}
	) as TlsConnectionOptions;
	const mergedTls = {
	...(tlsFromEnv ?? {}),
	...(userRedisOptions.tls as TlsConnectionOptions | undefined ?? {}),
	...(redisOpts.tls as TlsConnectionOptions | undefined ?? {})
	} as TlsConnectionOptions;

[Copilot]

The conditional check if (firstClusterNode) on line 172 is redundant since clusterNodes[0] is already checked for truthiness on line 170 (clusterNodes && clusterNodes.length). An empty array would have falsy length, so firstClusterNode will always be defined when this code is reached.

Suggested change

	const firstClusterNode = clusterNodes[0];
	const nodeCredentials: Partial<RedisOptions> = firstClusterNode
	? redisOptsFromUrl(firstClusterNode)
	: {};
	const nodeCredentials: Partial<RedisOptions> = redisOptsFromUrl(clusterNodes[0]);

[Copilot]

Using as any casting and Record<string, any> weakens type safety. Consider defining a proper type for the nested redisOptions property or using TypeScript's utility types to extract the correct type from ioredis ClusterOptions.

[Copilot]

[nitpick] The credential merging logic (lines 181-199) and TLS merging logic (lines 201-219) are repetitive. Consider extracting a generic helper function like mergeRedisOption(optionKey, ...sources) to reduce code duplication and improve maintainability.

[manast]

@copilot open a new pull request to apply changes based on this feedback

[Copilot]

The TLS certificate is decoded from base64 to ASCII. However, PEM-formatted certificates are already base64-encoded text. This double conversion may corrupt the certificate. Consider using .toString('utf8') instead of .toString('ascii'), or if the environment variable already contains a PEM certificate, no base64 decoding may be needed.

Suggested change

	).toString("ascii"),
	).toString("utf8"),