feat(RedisBroker): discordjs/core gateway impl (#11005)

* feat(RedisBroker): discordjs/core gateway impl

* fix: import

Co-authored-by: Almeida <github@almeidx.dev>

* chore: iterate

* fix: publish properly

* chore: deps and docs

* chore: leftover comment

* chore: suggested changes

* doc: bad comment

---------

Co-authored-by: Almeida <github@almeidx.dev>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

RFC-Caching-Layer.md

@@ -0,0 +1,212 @@
+<!-- RFC: Async Caching Layer & Modernized Manager Pattern for `@discordjs/next` -->
+
+# 1. Introduction
+
+Key supported use cases & goals:
+
+- **Zero Caching:** The library must function perfectly even with caching completely disabled
+- **Async, Remote Caching:** Support for external, asynchronous data stores like Redis
+- **In-Memory Caching:** The traditional `Collection`-based synchronous cache (default)
+- **Complete Flexibility**: Absolute user control over what is cached and for how long, without any internal assumptions or limitations
+
+I ask readers to address the dedicated "Problems" sections in each part of the proposal, as well as any other general
+concerns.
+
+Also of note, I'm aware that we're going to be using some sort of dependency injection container. For the sake of simplicity,
+this proposal just takes in the `Client` everywhere.
+
+# 2. Design Principles
+
+1. **Async-First:** All cache operations must return an `Awaitable<T>`
+2. **Fetch-First:** Users should use `manager.fetch(id)` to get data. The manager will check the cache first, and if it misses
+   (or if caching is disabled), it will hit the API
+3. **Map-like:** The cache interface should remain map-like, as it currently is in mainlib
+
+# 3. The Cache Interface
+
+At this time, I think those are the primary methods that need implemented. They are most certainly enough for most use
+cases. Some considerations around being able to iterate on the data should be made.
+
+## Suggested Interface
+
+```ts
+import type { Awaitable } from '@discordjs/util';
+import type { Structure } from '@discordjs/structures';
+import type { Snowflake } from 'discord-api-types/globals';
+
+export type RawAPIType<T> = T extends Structure<infer Raw, any> ? Raw : never;
+
+export type StructureCreator<
+	Value extends Structure<{ id: Snowflake }>,
+	Raw extends RawAPIType<Value> = RawAPIType<Value>,
+> = (data: Partial<Raw> & { id: Snowflake }) => Value;
+
+export interface Cache<Value extends Structure<{ id: Snowflake }>, Raw extends RawAPIType<Value> = RawAPIType<Value>> {
+	/**
+	 * The function used to construct instances of the structure this cache holds.
+	 */
+	readonly construct: StructureCreator<Value, Raw>;
+
+	/**
+	 * Retrieves an item from the cache.
+	 */
+	get(key: Snowflake): Awaitable<Value | undefined>;
+
+	/**
+	 * Sets an item in the cache.
+	 */
+	set(key: Snowflake, value: Value): Awaitable<void>;
+
+	/**
+	 * Adds or updates data in the cache, returning the instantiated structure.
+	 * If the item exists, it patches it with the new data unless `overwrite` is true.
+	 * If it does not exist, it constructs a new instance and stores it.
+	 */
+	add(data: Partial<Raw> & { id: Snowflake }, overwrite?: boolean): Awaitable<Value>;
+
+	/**
+	 * Checks if an item exists in the cache.
+	 */
+	has(key: Snowflake): Awaitable<boolean>;
+
+	/**
+	 * Deletes an item from the cache.
+	 */
+	delete(key: Snowflake): Awaitable<boolean>;
+
+	/**
+	 * Clears all items from the cache.
+	 */
+	clear(): Awaitable<void>;
+
+	/**
+	 * Gets the number of items in the cache.
+	 */
+	getSize(): Awaitable<number>;
+}
+```
+
+[^1]
+
+## Problems
+
+- Consider use cases that require iterating over cached items. `keys()`, `values()`, and `entries()` could be added,
+  but they raise some type questions. They probably have to return `AsyncIterable | Iterable`?
+- It's possible there's an oversight with `add` being so highly-responsible (i.e. functioning as more of an upsert).
+
+# 4. The Manager Pattern
+
+## Suggested implementation
+
+```ts
+import type { Client } from '../client/Client.js';
+import type { Cache, RawAPIType } from '../cache/Cache.js';
+import type { Structure } from '@discordjs/structures';
+import type { Snowflake } from 'discord-api-types/globals';
+
+export abstract class BaseManager<
+	Value extends Structure<{ id: Snowflake }>,
+	Raw extends RawAPIType<Value> = RawAPIType<Value>,
+> {
+	public constructor(
+		public readonly client: Client,
+		public readonly cache: Cache<Value, Raw>,
+	) {}
+
+	// Child classes provide methods specific to the endpoints they manage.
+}
+```
+
+## Problems
+
+- Considering `_add` is now notably absent, we can potentially justify just having an interface rather than a base class.
+- Something to consider is that perhaps `StructureCreator` should be on the Manager rather than the Cache.
+  I handled it this way to avoid the `Cache` needing a back-reference to the Manager, but we do end up with cases
+  where Manager code has to `this.cache.construct`. Perhaps an option is for both the `Manager` and `Cache` to have it.
+
+## Specific manager: UserManager
+
+The `UserManager` implements the `fetch` method, which is the primary way users will interact with data.
+
+```ts
+import { BaseManager } from './BaseManager.js';
+import { User } from '@discordjs/structures';
+import type { APIUser } from 'discord-api-types/v10';
+import type { Snowflake } from 'discord-api-types/globals';
+
+export interface FetchOptions {
+	/**
+	 * Whether to skip the cache check and force an API request.
+	 */
+	force?: boolean;
+	/**
+	 * Whether to cache the fetched result.
+	 */
+	cache?: boolean;
+}
+
+export class UserManager extends BaseManager<User, APIUser> {
+	/**
+	 * Fetches a user by ID.
+	 * Checks the cache first unless `force` is true.
+	 */
+	public async fetch(id: Snowflake, options: FetchOptions = {}): Promise<User> {
+		const { force = false, cache = true } = options;
+
+		if (!force) {
+			const cached = await this.cache.get(id);
+			if (cached) {
+				return cached;
+			}
+		}
+
+		// Cache miss or forced fetch. Hit the API.
+		// Assuming `this.client.api` is an instance of `@discordjs/core` API
+		const rawUser = await this.client.api.users.get(id);
+
+		if (!cache) {
+			return this.cache.construct(rawUser);
+		}
+
+		// Pass the raw data to the cache to handle caching and instantiation.
+		// We pass overwrite=true because we know this is a fresh, complete object from the API.
+		return await this.cache.add(rawUser, true);
+	}
+
+	// Other methods relevant to /users endpoints
+}
+```
+
+## Note on `_add`
+
+In the old implementation, `BaseManager` had an `_add` method responsible for instantiating structures and
+patching existing ones. In this new design, that responsibility has been entirely shifted to the `Cache` interface.
+
+# 5. Usage examples
+
+```ts
+// Userland code. Notably what you would do today
+const user = await client.users.fetch('123456789012345678');
+console.log(user.username);
+
+// A difference that will very quickly & obviously pop up is around structures we normally guaranteed were cached:
+const channel = await client.channels.fetch(process.env.SOME_CHANNEL_ID);
+console.log(channel.name);
+
+// Gateway Event Example (Internal Library Code)
+client.ws.on('USER_UPDATE', async (partialData) => {
+	// partialData might only contain { id: '...', username: 'new_name' }
+	// A great aspect about this is that we have no cloning logic here! Patching logic under the hood does not mutate.
+	// A drawback is that `add` will invoke `.get` internally as well. Might want to consider some method of optimization here.
+	const existingUser = await client.users.cache.get(partialData.id);
+	// Note: If the user is not in the cache, this will construct and cache a partial User structure.
+	const updatedUser = await client.users.cache.add(partialData);
+	if (existingUser) {
+		client.emit('userUpdate', existingUser, updatedUser);
+	}
+});
+```
+
+# Footnotes
+
+[^1] Derived from some existing work/PoC done in #10983

packages/brokers/package.json

@@ -67,6 +67,8 @@
 	"homepage": "https://discord.js.org",
 	"funding": "https://github.com/discordjs/discord.js?sponsor",
 	"dependencies": {
+		"@discordjs/core": "workspace:^",
+		"@discordjs/ws": "workspace:^",
 		"@msgpack/msgpack": "^3.1.3",
 		"@vladfrangu/async_event_emitter": "^2.4.7",
 		"ioredis": "^5.9.3"

packages/brokers/src/brokers/redis/RedisGateway.ts

@@ -0,0 +1,138 @@
+import type { Gateway, GatewayDispatchPayload, GatewaySendPayload, GatewayDispatchEvents } from '@discordjs/core';
+import type { ManagerShardEventsMap, WebSocketShardEvents } from '@discordjs/ws';
+import { AsyncEventEmitter } from '@vladfrangu/async_event_emitter';
+import type { PubSubRedisBroker } from './PubSubRedis.js';
+
+export type DiscordEvents = {
+	[K in GatewayDispatchEvents]: GatewayDispatchPayload & {
+		t: K;
+	};
+};
+
+interface BrokerProps<Payload> {
+	payload: Payload;
+	shardId: number;
+}
+
+interface Events extends DiscordEvents {
+	// eslint-disable-next-line @typescript-eslint/no-use-before-define
+	[RedisGateway.GatewaySendEvent]: GatewaySendPayload;
+}
+
+export type RedisBrokerDiscordEvents = {
+	[K in keyof Events]: BrokerProps<Events[K]>;
+};
+
+/**
+ * RedisGateway is an implementation for core's Gateway interface built on top of our Redis brokers.
+ *
+ * Some important notes:
+ * - Instances for this class are for your consumers/services that need the gateway. Naturally, the events passed into
+ * `init` are the only ones the core client will be able to emit
+ * - You can also opt to use the class as-is without `@discordjs/core`, if you so desire. Events are properly typed
+ * - You need to implement your own gateway service. Refer to the example below for how that would look like. This class
+ * offers some static methods and properties that help in this errand. It is extremely important that you `publish`
+ * events as the receiving service expects, and also that you handle GatewaySend events.
+ * - One drawback to using this directly with `@discordjs/core` is that you lose granular control over when to `ack`
+ * events. This implementation `ack`s as soon as the event is emitted to listeners. In practice, this means that if your
+ * service crashes while handling an event, it's pretty arbitrary wether that event gets re-processed on restart or not.
+ * (Mostly dependant on if your handler is async or not, and also if the `ack` call has time to go through).
+ *
+ * @example
+ * ```ts
+ * // gateway-service/index.ts
+ * import { RedisGateway, PubSubRedisBroker, kUseRandomGroupName } from '@discordjs/brokers';
+ * import Redis from 'ioredis';
+ *
+ * // the `name` here probably should be env-determined if you need to scale this. see the main README for more information.
+ * // also, we use a random group because we do NOT want work-balancing on gateway_send events.
+ * const broker = new PubSubRedisBroker(new Redis(), { group: kUseRandomGroupName, name: 'send-consumer-1' });
+ * const gateway = new WebSocketManager(gatewayOptionsHere); // see @discordjs/ws for examples.
+ *
+ * // emit events over the broker
+ * gateway.on(WebSocketShardEvents.Dispatch, (...data) => void broker.publish(...RedisGateway.toPublishArgs(data)));
+ *
+ * // listen to payloads we should send to Discord
+ * broker.on(RedisGateway.GatewaySendEvent, async ({ data: { payload, shardId }, ack }) => {
+ * 	await gateway.send(shardId, payload);
+ * 	await ack();
+ * });
+ * await broker.subscribe([RedisGateway.GatewaySendEvent]);
+ * await gateway.connect();
+ * ```
+ *
+ * ```ts
+ * // other-service/index.ts
+ * import { RedisGateway, PubSubRedisBroker, kUseRandomGroupName } from '@discordjs/brokers';
+ * import Redis from 'ioredis';
+ *
+ * // the name here should absolutely be env-determined, see the main README for more information.
+ * const broker = new PubSubRedisBroker(new Redis(), { group: 'my-service-name', name: 'service-name-instance-1' });
+ * // unfortunately, we have to know the shard count. ideally this should be an env var
+ * const gateway = new RedisGateway(broker, Number.parseInt(process.env.SHARD_COUNT, 10));
+ *
+ * const rest = new REST({ version: '10' }).setToken(process.env.DISCORD_TOKEN);
+ * const client = new Client({ rest, gateway });
+ *
+ * // set up your client as you normally would with core
+ *
+ * // subscribe to the events that you want
+ * await gateway.init([GatewayDispatchEvents.GuildCreate, GatewayDispatchEvents.MessageCreate]);
+ * ```
+ */
+export class RedisGateway
+	extends AsyncEventEmitter<{ dispatch: ManagerShardEventsMap[WebSocketShardEvents.Dispatch] }>
+	implements Gateway
+{
+	/**
+	 * Event used over the broker used to tell shards to send a payload to Discord.
+	 */
+	public static readonly GatewaySendEvent = 'gateway_send' as const;
+
+	/**
+	 * Converts a dispatch event from `@discordjs/ws` to arguments for a `broker.publish` call.
+	 */
+	public static toPublishArgs(
+		data: ManagerShardEventsMap[WebSocketShardEvents.Dispatch],
+	): [GatewayDispatchEvents, BrokerProps<GatewayDispatchPayload>] {
+		const [payload, shardId] = data;
+		return [payload.t, { shardId, payload }];
+	}
+
+	public constructor(
+		private readonly broker: PubSubRedisBroker<RedisBrokerDiscordEvents>,
+		private readonly shardCount: number,
+	) {
+		super();
+	}
+
+	public getShardCount(): number {
+		return this.shardCount;
+	}
+
+	public async send(shardId: number, payload: GatewaySendPayload): Promise<void> {
+		await this.broker.publish(RedisGateway.GatewaySendEvent, { payload, shardId });
+	}
+
+	public async init(events: GatewayDispatchEvents[]) {
+		for (const event of events) {
+			// async_event_emitter nukes our types on this one.
+			this.broker.on(
+				event,
+				({
+					ack,
+					data: { payload, shardId },
+				}: {
+					// eslint-disable-next-line @typescript-eslint/method-signature-style
+					ack: () => Promise<void>;
+					data: BrokerProps<GatewayDispatchPayload>;
+				}) => {
+					this.emit('dispatch', payload, shardId);
+					void ack();
+				},
+			);
+		}
+
+		await this.broker.subscribe(events);
+	}
+}

packages/brokers/src/index.ts

@@ -1,5 +1,6 @@
 export * from './brokers/redis/BaseRedis.js';
 export * from './brokers/redis/PubSubRedis.js';
+export * from './brokers/redis/RedisGateway.js';
 export * from './brokers/redis/RPCRedis.js';
 
 export * from './brokers/Broker.js';

pnpm-lock.yaml

@@ -683,6 +683,12 @@ importers:
 
   packages/brokers:
     dependencies:
+      '@discordjs/core':
+        specifier: workspace:^
+        version: link:../core
+      '@discordjs/ws':
+        specifier: workspace:^
+        version: link:../ws
       '@msgpack/msgpack':
         specifier: ^3.1.3
         version: 3.1.3
@@ -5737,6 +5743,7 @@ packages:
   '@smithy/middleware-endpoint@4.4.16':
     resolution: {integrity: sha512-L5GICFCSsNhbJ5JSKeWFGFy16Q2OhoBizb3X2DrxaJwXSEujVvjG9Jt386dpQn2t7jINglQl0b4K/Su69BdbMA==}
     engines: {node: '>=18.0.0'}
+    deprecated: Please upgrade to @smithy/middleware-endpoint@4.1.15 or higher to fix a bug preventing the resolution of ENV and config file custom endpoints https://github.com/smithy-lang/smithy-typescript/issues/1645
 
   '@smithy/middleware-retry@4.4.33':
     resolution: {integrity: sha512-jLqZOdJhtIL4lnA9hXnAG6GgnJlo1sD3FqsTxm9wSfjviqgWesY/TMBVnT84yr4O0Vfe0jWoXlfFbzsBVph3WA==}