fix: Check left guilds & clear expected guilds - #11547
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a GuildDeleteAction that removes guild channels, destroys voice adapters, deletes the guild from cache, and emits GuildDelete; registers the action and calls it from GUILD_DELETE (unavailable) and READY handlers. expectedGuilds is changed to a per-shard Collection to track pending guilds. ChangesGuild Deletion Flow
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/discord.js/src/client/websocket/handlers/READY.js`:
- Around line 24-27: The loop is evicting stale guilds by calling
client.actions.GuildDelete.handle(guild) directly so no public event is emitted;
change it to emit the public guild delete event (Events.GuildDelete) so
consumers observe the removal and still run the internal handler—i.e., for each
guild in client.guilds.cache not in client.expectedGuilds, call
client.emit(Events.GuildDelete, guild) (or call the action then emit if internal
logic must run first) instead of invoking client.actions.GuildDelete.handle
directly; locate the loop referencing client.guilds.cache,
client.expectedGuilds, and client.actions.GuildDelete.handle to apply this
change.
- Around line 17-27: The READY handler is clobbering client.expectedGuilds for
all shards and deleting guilds from client.guilds.cache regardless of their
shard; instead, scope expected-guild maintenance and stale-guild cleanup to the
current shardId: keep per-shard expected sets (e.g., client.expectedGuilds as a
Map keyed by shardId or store a Set at client.expectedGuilds.get(shardId)),
populate only that shard's Set when iterating data.guilds and set guild.shardId
= shardId before client.guilds._add(guild), and when scanning for stale guilds
only consider cached guilds with guild.shardId === shardId before calling
client.actions.GuildDelete.handle(guild). Ensure you do not clear the global
structure unconditionally and only remove entries for the current shard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e6f2141a-59cd-4cbf-beb8-87e882e366fd
📒 Files selected for processing (4)
packages/discord.js/src/client/actions/ActionsManager.jspackages/discord.js/src/client/actions/GuildDelete.jspackages/discord.js/src/client/websocket/handlers/GUILD_DELETE.jspackages/discord.js/src/client/websocket/handlers/READY.js
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-03-30T11:04:39.419Z
Learnt from: almeidx
Repo: discordjs/discord.js PR: 11471
File: packages/discord.js/src/util/Util.js:83-86
Timestamp: 2026-03-30T11:04:39.419Z
Learning: For the discord.js and rest packages, follow the existing convention for constructing request URLs: use the `api` base URL and `version` string via raw interpolation (e.g., `${api}/v${version}${route}`) without adding normalization such as trimming trailing slashes on `api` or stripping/adding/remapping a leading `v` on `version`. Do not recommend changing this behavior during code review unless the existing pattern in this repo is intentionally being replaced.
Applied to files:
packages/discord.js/src/client/websocket/handlers/GUILD_DELETE.jspackages/discord.js/src/client/actions/ActionsManager.jspackages/discord.js/src/client/actions/GuildDelete.jspackages/discord.js/src/client/websocket/handlers/READY.js
📚 Learning: 2026-05-18T13:40:11.014Z
Learnt from: almeidx
Repo: discordjs/discord.js PR: 11530
File: packages/core/src/api/user.ts:190-190
Timestamp: 2026-05-18T13:40:11.014Z
Learning: When reviewing discord.js (and related) source files, JSDoc `see` links that point to Discord’s documentation on `https://docs.discord.com/developers/...` are correct and should not be flagged as inconsistent. For new/updated links going forward, prefer `https://docs.discord.com/developers` over the legacy `https://discord.com/developers/docs` domain.
Applied to files:
packages/discord.js/src/client/websocket/handlers/GUILD_DELETE.jspackages/discord.js/src/client/actions/ActionsManager.jspackages/discord.js/src/client/actions/GuildDelete.jspackages/discord.js/src/client/websocket/handlers/READY.js
📚 Learning: 2026-05-22T17:29:55.128Z
Learnt from: kshitijanurag
Repo: discordjs/discord.js PR: 11537
File: packages/discord.js/src/structures/Guild.js:1486-1495
Timestamp: 2026-05-22T17:29:55.128Z
Learning: When handling Discord Gateway opcode `RequestChannelInfo` (opcode name: `RequestChannelInfo` / payload gateway opcode), do not add or suggest a `nonce` field for correlating responses. This opcode’s payload supports only `guild_id` and `fields` (e.g., `fields: ["status", "voice_start_time"]`); ensure any review feedback reflects that schema and treats `nonce` as unsupported for this opcode.
Applied to files:
packages/discord.js/src/client/websocket/handlers/GUILD_DELETE.jspackages/discord.js/src/client/actions/ActionsManager.jspackages/discord.js/src/client/actions/GuildDelete.jspackages/discord.js/src/client/websocket/handlers/READY.js
🔇 Additional comments (3)
packages/discord.js/src/client/actions/GuildDelete.js (1)
5-16: LGTM!packages/discord.js/src/client/actions/ActionsManager.js (1)
22-22: LGTM!packages/discord.js/src/client/websocket/handlers/GUILD_DELETE.js (1)
25-25: LGTM!
Qjuh
left a comment
Qjuh
left a comment
There was a problem hiding this comment.
The logic for the calling the GuildDelete action if missing from READY event looks fine, but I disagree with the other change since I don't see a benefit from separating the expectedGuilds by shardId.
This is necessary if the connection suddenly breaks when not all expected guilds have been received in |
This bug doesn't occur in v14, as the cache is cleared in |
Ah, that explains the need for the split, thank you. But that sounds like the other half either can't happen or would exist on v14 anyway, as v14 doesn't do those artificial GUILD_DELETE events on reconnect. Guess it needs someone to test if the issue even happens on v14 / if only the split is implemented in v15. |
I can do this. |
|
So, I ran some testing.
|
Afaict the only case that would be relevant is if Client is used without |
If connect without the 
|
This reverts commit 3d01513.
This reverts commit 210f9a9.
2 participants
This PR adds a check for cached guilds when receiving a gateway READY event.
If a cached guild is no longer available (i. e., has left) and is not sent in a gateway READY event, it is removed from the cache.
The
expectedGuildshas been refactored and is now split into shards.Fix #11546