Fix/invite handlers null guild#11475
Conversation
`resolveBody()` crashes with `TypeError: Cannot read properties of undefined (reading 'push')` when `options.attachments` is provided without `options.files`, because `files?.map()` returns `undefined` and `.push()` is then called on it. Default the result to an empty array with `?? []`.
INVITE_CREATE and INVITE_DELETE handlers crash with `TypeError: Cannot read properties of undefined (reading 'invites')` when the guild is not in cache, unlike other handlers (e.g. GUILD_BAN_ADD) which correctly guard with `if (!guild) return;`.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThree defensive programming fixes across the discord.js library: guard conditions added to INVITE_CREATE and INVITE_DELETE websocket handlers to check guild existence, and initialization logic in MessagePayload adjusted to ensure attachments array is always defined. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/discord.js/src/structures/MessagePayload.js`:
- Around line 193-200: The attachments mapping in MessagePayload (the
this.options.files?.map block) builds attachment objects without using
file.toJSON(), so custom IDs set via AttachmentBuilder.setId() are ignored;
change the mapper to call file.toJSON() when available and spread its result
into the attachment object after the index-based id (e.g., create object with
id: index.toString() and then ...file.toJSON() or ...file.toJSON?.()) so any
custom id from AttachmentBuilder overrides the default index id while still
falling back to existing fields when toJSON() is not present.
🪄 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: db093f59-2266-4cbe-8bcf-17eb29a379a8
📒 Files selected for processing (3)
packages/discord.js/src/client/websocket/handlers/INVITE_CREATE.jspackages/discord.js/src/client/websocket/handlers/INVITE_DELETE.jspackages/discord.js/src/structures/MessagePayload.js
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Qjuh
Repo: discordjs/discord.js PR: 11423
File: packages/discord.js/src/structures/MessagePayload.js:193-206
Timestamp: 2026-02-16T18:45:40.408Z
Learning: In discord.js MessagePayload.js, when mapping files to attachments, custom ids set via AttachmentBuilder.setId() should override the index-based id. The spread operator should come after the explicit id assignment to allow toJSON() values to take precedence.
📚 Learning: 2026-02-16T18:45:40.408Z
Learnt from: Qjuh
Repo: discordjs/discord.js PR: 11423
File: packages/discord.js/src/structures/MessagePayload.js:193-206
Timestamp: 2026-02-16T18:45:40.408Z
Learning: In discord.js MessagePayload.js, when mapping files to attachments, custom ids set via AttachmentBuilder.setId() should override the index-based id. The spread operator should come after the explicit id assignment to allow toJSON() values to take precedence.
Applied to files:
packages/discord.js/src/structures/MessagePayload.js
📚 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/structures/MessagePayload.jspackages/discord.js/src/client/websocket/handlers/INVITE_CREATE.jspackages/discord.js/src/client/websocket/handlers/INVITE_DELETE.js
📚 Learning: 2026-03-10T13:07:50.826Z
Learnt from: Qjuh
Repo: discordjs/discord.js PR: 11443
File: packages/discord.js/src/structures/DMChannel.js:83-94
Timestamp: 2026-03-10T13:07:50.826Z
Learning: In discord.js (packages/discord.js/src/structures/DMChannel.js), `DMChannel#recipientId` intentionally returns `null` for DMChannels that do not include the client user (e.g. DMs between two non-client users observed via user-installed apps). The methods `fetch()` and `toString()` use `recipientId` unconditionally; calling them on a non-client DMChannel is considered a caller error and is intentional API design — not a bug to guard against.
Applied to files:
packages/discord.js/src/client/websocket/handlers/INVITE_CREATE.jspackages/discord.js/src/client/websocket/handlers/INVITE_DELETE.js
🔇 Additional comments (3)
packages/discord.js/src/client/websocket/handlers/INVITE_DELETE.js (1)
9-9: LGTM! Proper defensive guard to prevent crash.The combined check for both
channelandguildbefore proceeding is correct. Line 14 accessesguild.invites.cache.delete()which would throw ifguildis undefined. This follows the established pattern used in other handlers likeVOICE_STATE_UPDATEandTHREAD_LIST_SYNC.packages/discord.js/src/client/websocket/handlers/INVITE_CREATE.js (1)
8-8: LGTM! Consistent with the INVITE_DELETE fix.The guard ensures
guild.invites._add()on line 11 won't throw when the guild isn't cached. This mirrors the fix inINVITE_DELETE.jsand aligns with the pattern in other handlers likeTHREAD_LIST_SYNC.jsandVOICE_STATE_UPDATE.js.packages/discord.js/src/structures/MessagePayload.js (1)
193-200: Fix correctly prevents the crash but introduces an unintended behavioral change for message edits.The
?? []fallback properly fixes the crash whenoptions.attachmentsis provided withoutoptions.files. However, this changes the payload from omittingattachmentsto includingattachments: []when neitherfilesnorattachmentsoptions are provided.Per Discord API semantics documented in the codebase (MessageManager.js:217-218: "All attachments will be kept if omitted"), an empty attachments array signals "remove all attachments," whereas omitting the field preserves existing attachments. This can unintentionally strip attachments from messages when users edit only content or embeds.
Verify whether the empty array behavior is intentional, or whether
attachmentsshould be conditionally included only when explicitly set or non-empty.
| const attachments = | ||
| this.options.files?.map((file, index) => ({ | ||
| id: index.toString(), | ||
| description: file.description, | ||
| title: file.title, | ||
| waveform: file.waveform, | ||
| duration_secs: file.duration, | ||
| })) ?? []; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Custom file IDs from AttachmentBuilder.setId() may not propagate to attachment metadata.
The mapping extracts fields explicitly without calling toJSON() on files, so custom IDs set via AttachmentBuilder.setId() won't override the index-based id. Based on learnings, custom IDs should take precedence over index-based assignment.
If files can be AttachmentBuilder instances with custom IDs, consider:
Allow toJSON() values to override explicit fields
const attachments =
this.options.files?.map((file, index) => ({
id: index.toString(),
description: file.description,
title: file.title,
waveform: file.waveform,
duration_secs: file.duration,
+ ...(isJSONEncodable(file) ? file.toJSON() : {}),
})) ?? [];Based on learnings: "In discord.js MessagePayload.js, when mapping files to attachments, custom ids set via AttachmentBuilder.setId() should override the index-based id. The spread operator should come after the explicit id assignment to allow toJSON() values to take precedence."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/discord.js/src/structures/MessagePayload.js` around lines 193 - 200,
The attachments mapping in MessagePayload (the this.options.files?.map block)
builds attachment objects without using file.toJSON(), so custom IDs set via
AttachmentBuilder.setId() are ignored; change the mapper to call file.toJSON()
when available and spread its result into the attachment object after the
index-based id (e.g., create object with id: index.toString() and then
...file.toJSON() or ...file.toJSON?.()) so any custom id from AttachmentBuilder
overrides the default index id while still falling back to existing fields when
toJSON() is not present.
There was a problem hiding this comment.
The changes to this file should be reverted
|
Did you actually test this and received that error? Or are you assuming it would? Because there should be no way to have a guild channel in cache when the Guild itself isn't cached. So the |
|
No I am sorry I should have, I didn't test it. I started reading the repo and tried to find a few things that need fixing or help out, as I started using it. Will make sure to create a dummy app and test it in the future. |
3 participants
INVITE_CREATEandINVITE_DELETEgateway handlers crash withTypeError: Cannot read properties of undefined (reading 'invites')when the guild is not in cache.Other handlers like
GUILD_BAN_ADDcorrectly guard withif (!guild) return;.