Better type gaurds on channels and refactored Message class for future testing#20
Better type gaurds on channels and refactored Message class for future testing#20blstmo merged 5 commits intofluxerjs:mainfrom
Conversation
…it could confuse people thinking it does permission checks
…ture, also added check for empty messages
|
@MusicMakerOwO is attempting to deploy a commit to the blstmo's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis refactoring standardizes message sending across Client, Message, and Webhook classes by extracting payload creation into reusable helpers, updates Channel type guards to use type predicates instead of booleans, and constrains MessageSendOptions to object-only input rather than accepting direct strings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 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)
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 the current code and only fix it if needed.
Inline comments:
In `@packages/fluxer-core/src/structures/Message.ts`:
- Around line 180-196: The _createMessageBody function currently only blocks
empty string inputs; update it to validate object payloads after calling
buildSendBody so truly empty bodies (e.g. {}, { content: '' }, { embeds: [] })
throw the same RangeError: after const base = buildSendBody(content), check that
base has at least one meaningful field (e.g. non-empty base.content or
base.embeds?.length or base.components?.length or other content-bearing keys)
and throw RangeError('Cannot send an empty message') if none are present; also
change the returned object construction to omit files when undefined using the
same conditional pattern as other callers (use files?.length ? { files, body } :
{ body } for both referenced_message and non-referenced branches). Ensure
changes reference _createMessageBody, buildSendBody, resolveMessageFiles, and
MessagePayload.
| async sendToChannel( | ||
| channelId: string, | ||
| payload: string | { content?: string; embeds?: APIEmbed[] }, | ||
| content: string | { content?: string; embeds?: APIEmbed[] }, | ||
| ): Promise<Message> { | ||
| const payload = await Message._createMessageBody(content); | ||
| return this.channels.send(channelId, payload); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "class ChannelManager" packages/fluxer-core/src/client/ChannelManager.ts
rg -n "send\s*\(" -C3 packages/fluxer-core/src/client/ChannelManager.ts
rg -n "MessagePayload|SendBodyResult|files" packages/fluxer-core/src/client/ChannelManager.tsRepository: fluxerjs/core
Length of output: 1902
🏁 Script executed:
rg -n "class Message" packages/fluxer-core/src
rg -n "_createMessageBody" -A5 packages/fluxer-core/src/client/Message.ts
rg -n "type MessagePayload|interface MessagePayload|type MessageSendOptions|interface MessageSendOptions" packages/fluxer-core/srcRepository: fluxerjs/core
Length of output: 913
🏁 Script executed:
cat -n packages/fluxer-core/src/structures/Message.ts | head -80
rg -n "_createMessageBody" -A10 packages/fluxer-core/src/structures/Message.tsRepository: fluxerjs/core
Length of output: 4662
🏁 Script executed:
sed -n '181,210p' packages/fluxer-core/src/structures/Message.ts
rg -n "type MessageSendOptions|interface MessageSendOptions" -A10 packages/fluxer-core/src/util/messageUtils.tsRepository: fluxerjs/core
Length of output: 1668
Fix type mismatch: sendToChannel passes MessagePayload but ChannelManager.send expects MessageSendOptions
Line 289 passes a MessagePayload ({ files, body }) to channels.send(), but ChannelManager.send() expects MessageSendOptions ({ content?, embeds?, files?, attachments? }). These are incompatible types. ChannelManager.send() calls buildSendBody(payload) and accesses opts.files, neither of which work correctly with the MessagePayload structure being passed in. Either:
- Have
sendToChannel()extract and rebuildMessageSendOptionsbefore callingchannels.send(), or - Add an overload to
ChannelManager.send()that acceptsMessagePayloadand calls_send()directly instead of re-processing.
There was a problem hiding this comment.
Will fix tomorrow morning, build did not report this 💔
There was a problem hiding this comment.
@MusicMakerOwO Thanks for confirming! No worries—type mismatches like this can slip through if TypeScript's strict mode isn't fully enabled or if type assertions are masking the incompatibility. Looking forward to the fix tomorrow. Let me know if you'd like me to review the updated code once it's ready! 👍
| /** Exposed for testing purposes, use Message.reply() or send() for normal use */ | ||
| static async _createMessageBody( | ||
| content: string | MessageSendOptions, | ||
| referenced_message?: { channel_id: string; message_id: string; guild_id?: string }, | ||
| ): Promise<MessagePayload> { | ||
| if (typeof content === 'string') { | ||
| if (content.length === 0) { | ||
| throw new RangeError('Cannot send an empty message'); | ||
| } | ||
| content = { content }; | ||
| } | ||
| const base = buildSendBody(content); | ||
| const files = content.files?.length ? await resolveMessageFiles(content.files) : undefined; | ||
| return referenced_message | ||
| ? { files, body: { ...base, referenced_message } } | ||
| : { files, body: { ...base } }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for files property handling in REST client
rg -n "files" -g '*.ts' packages --max-count=50 | head -100Repository: fluxerjs/core
Length of output: 6909
🏁 Script executed:
#!/bin/bash
# Look for multipart/form-data handling
rg -n "multipart|form-data" -g '*.ts' packagesRepository: fluxerjs/core
Length of output: 450
🏁 Script executed:
#!/bin/bash
# Search for the REST client request handling
fd -e ts packages -path '*/rest*' -o -path '*/request*' | head -20Repository: fluxerjs/core
Length of output: 225
🏁 Script executed:
#!/bin/bash
# Check MessagePayload type definition
rg -n "type MessagePayload|interface MessagePayload" -g '*.ts' packagesRepository: fluxerjs/core
Length of output: 348
🏁 Script executed:
#!/bin/bash
# Look for buildSendBody and resolveMessageFiles functions
rg -n "buildSendBody|resolveMessageFiles" -g '*.ts' packagesRepository: fluxerjs/core
Length of output: 2906
🏁 Script executed:
cat -n packages/fluxer-core/src/structures/Message.ts | sed -n '39,50p'Repository: fluxerjs/core
Length of output: 509
🏁 Script executed:
cat -n packages/fluxer-core/src/util/messageUtils.ts | sed -n '92,115p'Repository: fluxerjs/core
Length of output: 1092
🏁 Script executed:
cat -n packages/rest/src/RequestManager.ts | sed -n '85,100p'Repository: fluxerjs/core
Length of output: 639
🏁 Script executed:
cat -n packages/fluxer-core/src/util/messageUtils.test.tsRepository: fluxerjs/core
Length of output: 2585
🏁 Script executed:
rg -n "Cannot send an empty|empty message" -g '*.ts' packagesRepository: fluxerjs/core
Length of output: 169
🏁 Script executed:
cat -n packages/rest/src/RequestManager.ts | sed -n '1,25p'Repository: fluxerjs/core
Length of output: 883
Block empty object payloads and omit undefined files
Line 185 only guards empty string inputs. {}, { content: '' }, or { embeds: [] } still pass through buildSendBody without validation and can send an empty message to the API. Additionally, this method always includes files in the response even when undefined, inconsistent with how other callers (Channel, Webhook, ChannelManager) conditionally omit it using the pattern files?.length ? { body, files } : { body }.
🛠️ Suggested fix
static async _createMessageBody(
content: string | MessageSendOptions,
referenced_message?: { channel_id: string; message_id: string; guild_id?: string },
): Promise<MessagePayload> {
if (typeof content === 'string') {
if (content.length === 0) {
throw new RangeError('Cannot send an empty message');
}
content = { content };
}
+ const hasContent = !!content.content?.length;
+ const hasEmbeds = !!content.embeds?.length;
+ const hasFiles = !!content.files?.length;
+ if (!hasContent && !hasEmbeds && !hasFiles) {
+ throw new RangeError('Cannot send an empty message');
+ }
const base = buildSendBody(content);
const files = content.files?.length ? await resolveMessageFiles(content.files) : undefined;
- return referenced_message
- ? { files, body: { ...base, referenced_message } }
- : { files, body: { ...base } };
+ const body = referenced_message ? { ...base, referenced_message } : { ...base };
+ return files ? { files, body } : { body };
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluxer-core/src/structures/Message.ts` around lines 180 - 196, The
_createMessageBody function currently only blocks empty string inputs; update it
to validate object payloads after calling buildSendBody so truly empty bodies
(e.g. {}, { content: '' }, { embeds: [] }) throw the same RangeError: after
const base = buildSendBody(content), check that base has at least one meaningful
field (e.g. non-empty base.content or base.embeds?.length or
base.components?.length or other content-bearing keys) and throw
RangeError('Cannot send an empty message') if none are present; also change the
returned object construction to omit files when undefined using the same
conditional pattern as other callers (use files?.length ? { files, body } : {
body } for both referenced_message and non-referenced branches). Ensure changes
reference _createMessageBody, buildSendBody, resolveMessageFiles, and
MessagePayload.
There was a problem hiding this comment.
Completely skipped my mind but the suggested fix seems fine to just merge in
There was a problem hiding this comment.
@MusicMakerOwO, thank you for confirming! I'll apply the suggested fix to add proper empty message validation and consistent file handling.
Task: Apply empty message validation fix to _createMessageBody
- Add validation for empty object payloads after buildSendBody call
- Update return statement to conditionally omit undefined files
- Ensure consistency with other callers (Channel, Webhook, ChannelManager)
💡 For code changes across multiple files, use @coderabbitai plan in an issue comment
2 participants
Description
Type of change
Checklist
pnpm run lint)pnpm run buildsuccessfullypnpm run testsuccessfullyBREAKING CHANGE: Renamed Channel.isSendable() to isTextBased() for more clarity
Summary by CodeRabbit
Release Notes
Refactor
Client.sendToChannel()parameter naming for clarity.Channel.isSendable()toChannel.isTextBased()for consistency.isDM(),isVoice(), and newisLink()channel detection methods, improving TypeScript type narrowing.Message.send()method signature for flexibility.Bug Fixes
Webhook.send()to correctly use preprocessed options.