Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gateway-expected-guilds-by-shard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@wolfstar/plugin-gateway": patch
---

Track the guilds `clientReady` waits for by shard, like discord.js: a shard's new `READY` now replaces the unavailable guilds its previous one listed instead of adding to them, so a guild the bot left between two `READY`s of a shard (a session invalidated during startup) no longer holds `clientReady` back until `waitGuildTimeout`. The client also stops collecting these guild IDs once `clientReady` fired, where every reconnect's `READY` used to add them to a set that was never emptied.
6 changes: 6 additions & 0 deletions .changeset/gateway-forward-ready-guild-delete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@wolfstar/plugin-gateway": patch
"@wolfstar/plugin-broker": patch
---

Forward the `guildDelete` of a guild a `READY` drops: when a `READY` no longer lists a cached guild (the bot left it while offline), `GatewayClient` now emits `dispatch` for the `GUILD_DELETE` it applies, after the cache write and with the cached guild as its state, before `guildDelete`. `forwardGatewayDispatches` publishes it like any other dispatch, so a `plugin-broker` worker's `guildDelete` listeners clean these guilds up too. The made-up dispatch carries `s: 0` and is not emitted as `raw`.
3 changes: 2 additions & 1 deletion packages/plugin-broker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ destroy a `GatewayClient`), exiting is left to your handler.
receives, under its type (e.g. `MESSAGE_CREATE`) with its data as payload. It forwards them on the
client's `dispatch` event, once they are written to the client's cache, so workers sharing a
[`@wolfstar/plugin-cache`](https://www.npmjs.com/package/@wolfstar/plugin-cache) Redis cache see the
state each dispatch left when they receive it:
state each dispatch left when they receive it. That includes the `GUILD_DELETE` the client emits on `dispatch`
for a cached guild a `READY` no longer lists (one the bot left while offline), so workers clean those up too:

```ts
import { createBroker, forwardGatewayDispatches } from "@wolfstar/plugin-broker";
Expand Down
37 changes: 37 additions & 0 deletions packages/plugin-broker/tests/gateway-replay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,43 @@ describe("replayGatewayDispatches", () => {
expect(dispatchedOnWorker).not.toHaveBeenCalled();
});

test("GIVEN a READY that no longer lists a cached guild THEN the worker's guildDelete gets the dropped guild", async () => {
const { broker, consumer, producer, worker } = setup();
forwardGatewayDispatches(producer, broker);
replayGatewayDispatches(consumer, worker);

const deleted: unknown[][] = [];
worker.on("guildDelete", (...args) => void deleted.push(args));

await consumer.start();
await feed(producer, "GUILD_CREATE", {
id: guildId,
name: "Wolf Den",
roles: [],
emojis: [],
stickers: [],
features: [],
channels: [],
threads: [],
members: [],
});
await feed(producer, "READY", {
v: 10,
user: author,
guilds: [],
session_id: "s",
resume_gateway_url: "wss://gateway.discord.gg",
shard: [0, 1],
application: { id: "266624760782258186", flags: 0 },
});

await vi.waitFor(() => expect(deleted).toHaveLength(1));
const [guild, data] = deleted[0] as [{ id: string; name: string } | null, { id: string }];
expect(guild?.id).toBe(guildId);
expect(guild?.name).toBe("Wolf Den");
expect(data).toEqual({ id: guildId });
});

test("GIVEN events THEN only those dispatch types are replayed", async () => {
const { broker, consumer, producer, worker } = setup();
forwardGatewayDispatches(producer, broker);
Expand Down
5 changes: 3 additions & 2 deletions packages/plugin-gateway/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,9 @@ exception: it is always emitted, since it sets `client.user` from the payload al

On `READY`, the cached guilds of that shard which `READY` no longer lists are dropped and emitted as
`guildDelete`: the bot left them while disconnected, or while the process was down with a
persistent cache, and Discord does not replay those removals. This is best effort: a failure (cache unreachable,
unknown shard count) is reported through `error` and keeps the remaining guilds.
persistent cache, and Discord does not replay those removals. Each drop also emits `dispatch` as a `GUILD_DELETE`
(with `s: 0`, and no `raw`), so a process replaying this client's dispatches sees these deletions too. This is best
effort: a failure (cache unreachable, unknown shard count) is reported through `error` and keeps the remaining guilds.

### Replaying dispatches on another process

Expand Down
45 changes: 35 additions & 10 deletions packages/plugin-gateway/src/GatewayClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,9 @@ export class GatewayClient extends Client {
readonly #token: string;

// The guilds `READY` listed as initially unavailable, whose `GUILD_CREATE` or `GUILD_DELETE` the client waits for
// before emitting `clientReady`, like discord.js's `Client#expectedGuilds`.
readonly #expectedGuilds = new Set<string>();
// before emitting `clientReady`, like discord.js's `Client#expectedGuilds`, by shard: a shard's new `READY` replaces
// its own, so a guild the bot left between two of them is not waited for until `waitGuildTimeout`.
readonly #expectedGuilds = new Map<number, Set<string>>();

// The fallback timer emitting `clientReady` even with guilds still unavailable, like discord.js's own
// `readyTimeout`. Cleared and rescheduled every time `#checkClientReady` runs short of triggering it.
Expand Down Expand Up @@ -1046,8 +1047,13 @@ export class GatewayClient extends Client {
const isReady = payload.t === GatewayDispatchEvents.Ready;
if (isReady) {
await this.reconcileGuilds(payload.d, shardId);
// The guilds this shard's `READY` listed as initially unavailable, awaited by `clientReady`.
for (const guild of payload.d.guilds) this.#expectedGuilds.add(guild.id);
// The guilds this shard's `READY` listed as initially unavailable, awaited by `clientReady`. Once it fired,
// nothing waits for them any more.
if (this.clientReadyTimestamp === null) {
const expected = new Set(payload.d.guilds.map((guild) => guild.id));
if (expected.size > 0) this.#expectedGuilds.set(shardId, expected);
else this.#expectedGuilds.delete(shardId);
}
}

let state: unknown;
Expand All @@ -1071,13 +1077,21 @@ export class GatewayClient extends Client {
} else if (
(payload.t === GatewayDispatchEvents.GuildCreate ||
payload.t === GatewayDispatchEvents.GuildDelete) &&
this.#expectedGuilds.delete(payload.d.id)
this.#forgetExpectedGuild(shardId, payload.d.id)
) {
await this.#checkClientReady();
}
}
}

// Stops waiting for a guild of a shard, returning whether it was waited for.
#forgetExpectedGuild(shardId: number, guildId: string): boolean {
const expected = this.#expectedGuilds.get(shardId);
if (!expected?.delete(guildId)) return false;
if (expected.size === 0) this.#expectedGuilds.delete(shardId);
return true;
}

// A shard resuming a stored session gets no `READY`, which is what sets `client.user` and runs the ready check. It has
// no guilds to wait for either: the cache already holds them.
async #readyAfterResume(shardId: number): Promise<void> {
Expand Down Expand Up @@ -1169,12 +1183,17 @@ export class GatewayClient extends Client {
}

/**
* Drops the cached guilds of a shard its `READY` no longer lists, and emits `guildDelete` for each of them.
* Drops the cached guilds of a shard its `READY` no longer lists, and emits `dispatch` and `guildDelete` for each of
* them.
*
* @remarks
* They are the guilds the bot left while it was disconnected, or while the process was down with a persistent
* cache. Discord does not replay those removals on a new session, so the cache would otherwise keep them forever.
*
* Each drop is handled as a `GUILD_DELETE` (with `s: 0`) Discord would have sent, `raw` aside: `dispatch` fires once
* the guild is gone from the cache, with the cached guild as its state, so forwarding `dispatch` to other processes
* (`@wolfstar/plugin-broker`'s `forwardGatewayDispatches`) carries these deletions too.
*
* It needs a guilds cache able to enumerate its entries, and is skipped without one.
*
* It is best effort: any failure (cache unreachable, unknown shard count) is reported through `error` and stops the
Expand Down Expand Up @@ -1204,17 +1223,23 @@ export class GatewayClient extends Client {
const count = data.shard?.[1] ?? this.#shardCount ?? (await this.gateway.getShardCount());
const listed = new Set(data.guilds.map((guild) => guild.id));
const shardCount = BigInt(count);
const action = this.actions.get(GatewayDispatchEvents.GuildDelete);
for (const id of cached) {
if (listed.has(id) || Number((BigInt(id) >> 22n) % shardCount) !== shardId) continue;

const guild = (await this.guilds.cache.get(id)) ?? null;
await applyGatewayDispatch(cache, {
// Taken through the steps `handleDispatch` takes for a `GUILD_DELETE` Discord sends, `raw` aside (Discord did
// not send this one): `dispatch` is what forwards it to the processes sharing the cache, such as a broker's
// workers, which would otherwise never see this guild go.
const payload: GatewayDispatchPayload = {
op: GatewayOpcodes.Dispatch,
s: 0,
t: GatewayDispatchEvents.GuildDelete,
d: { id },
});
this.emit("guildDelete", guild, { id });
};
const state = await action?.before(payload.d);
await applyGatewayDispatch(cache, payload);
this.emit("dispatch", payload, shardId, state);
await action?.handle(payload.d, state, shardId);
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/plugin-gateway/src/util/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ export interface GatewayEventMap {
* Not emitted for `INTERACTION_CREATE`, served by the HTTP endpoint, nor for a dispatch dropped because its cache
* write failed with {@link GatewayClientOptions.cacheFailure} set to `"skip"`.
*
* Also emitted, with `s: 0` and no `raw` before it, for the `GUILD_DELETE` the client makes up for a cached guild a
* `READY` no longer lists, see {@link GatewayClient.reconcileGuilds}.
*
* `state` is what the dispatch's handler read before the cache write, e.g. the cached message a `MESSAGE_UPDATE`
* replaces, or `undefined`. It is what {@link GatewayClient.serializeDispatchState} serializes for other processes.
*/
Expand Down
39 changes: 39 additions & 0 deletions packages/plugin-gateway/tests/client-ready.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,45 @@ describe("GatewayClient#clientReady", () => {
expect(calls).toHaveLength(1);
});

test("GIVEN a shard's new READY no longer lists a pending guild THEN it stops waiting for it", async () => {
const client = createClient({ waitGuildTimeout: 15_000 });
const calls: unknown[][] = [];
client.on(GatewayEvents.ClientReady, (...args) => calls.push(args));

await dispatch(client, GatewayDispatchEvents.Ready, ready([{ id: "10", unavailable: true }]));
expect(calls).toHaveLength(0);

// The shard identifies again (its session was invalidated) and the bot left guild 10 in between.
await dispatch(client, GatewayDispatchEvents.Ready, ready());

expect(calls).toHaveLength(1);
});

test("GIVEN a READY on one shard THEN it only replaces the pending guilds of that shard", async () => {
const client = createClient({ waitGuildTimeout: 15_000 });
const calls: unknown[][] = [];
client.on(GatewayEvents.ClientReady, (...args) => calls.push(args));

await dispatch(
client,
GatewayDispatchEvents.Ready,
ready([{ id: "10", unavailable: true }]),
0,
);
await dispatch(
client,
GatewayDispatchEvents.Ready,
ready([{ id: "11", unavailable: true }]),
1,
);
await dispatch(client, GatewayDispatchEvents.Ready, ready(), 1);
// Shard 0 still waits for guild 10.
expect(calls).toHaveLength(0);

await dispatch(client, GatewayDispatchEvents.GuildDelete, { id: "10" }, 0);
expect(calls).toHaveLength(1);
});

test("GIVEN it already fired THEN a later GUILD_CREATE does not fire it again", async () => {
const client = createClient();
const calls: unknown[][] = [];
Expand Down
42 changes: 42 additions & 0 deletions packages/plugin-gateway/tests/hardening.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,48 @@
expect(await client.guilds.cache.get(otherShardGuild)).toBeDefined();
});

test("GIVEN a cached guild READY no longer lists THEN dispatch is emitted for it once, after the cache write", async () => {
const client = createClient(createInMemoryCache(), { shardCount: 2 });
send(client, GatewayDispatchEvents.GuildCreate, guild("10"));
send(client, GatewayDispatchEvents.GuildCreate, guild("11"));
send(client, GatewayDispatchEvents.GuildCreate, guild(otherShardGuild), 1);
await client.idle();

const deletes: { payload: GatewayDispatchPayload; shardId: number; state: unknown }[] = [];
const cachedOnDispatch: unknown[] = [];
const raws: GatewayDispatchPayload[] = [];
const order: string[] = [];
client.on("raw", (payload) => void raws.push(payload));

Check warning on line 358 in packages/plugin-gateway/tests/hardening.test.ts

View workflow job for this annotation

GitHub Actions / 馃敔 Lint project

eslint(no-shadow)

packages/plugin-gateway/tests/hardening.test.ts:358:23: 'payload' is already declared in the upper scope.
client.on("dispatch", (payload, shardId, state) => {

Check warning on line 359 in packages/plugin-gateway/tests/hardening.test.ts

View workflow job for this annotation

GitHub Actions / 馃敔 Lint project

eslint(no-shadow)

packages/plugin-gateway/tests/hardening.test.ts:359:28: 'payload' is already declared in the upper scope.
if (payload.t !== GatewayDispatchEvents.GuildDelete) return;
order.push("dispatch");
deletes.push({ payload, shardId, state });
cachedOnDispatch.push(client.cache!.guilds!.get(payload.d.id));
});
client.on("guildDelete", () => void order.push("guildDelete"));

send(client, GatewayDispatchEvents.Ready, {
user,
guilds: [{ id: "10", unavailable: true }],
session_id: "s",
});
await client.idle();

expect(deletes).toHaveLength(1);
expect(deletes[0]!.payload).toEqual({
op: GatewayOpcodes.Dispatch,
s: 0,
t: GatewayDispatchEvents.GuildDelete,
d: { id: "11" },
});
expect(deletes[0]!.shardId).toBe(0);
expect((deletes[0]!.state as { name: string } | undefined)?.name).toBe("Guild 11");
expect(await cachedOnDispatch[0]).toBeUndefined();
expect(order).toEqual(["dispatch", "guildDelete"]);
// The made-up GUILD_DELETE is not something Discord sent.
expect(raws.map((raw) => raw.t)).toEqual([GatewayDispatchEvents.Ready]);
});

test("GIVEN an unreachable cache THEN READY is still emitted under the default skip policy", async () => {
const cache = createInMemoryCache();
// `createInMemoryCache` builds MemoryEntityCaches, which can enumerate their entries.
Expand Down
Loading