Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughChanged ephemeral message handling across commands from awaited to fire-and-forget and updated the Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Command Handler
participant Ephemeral as ephemeral()
participant Resolver as Message Resolver
participant API as modules.shared.api
Handler->>Ephemeral: call ephemeral( context.reply(...) ) (void - fire-and-forget)
Ephemeral->>Resolver: resolve MaybePromise<PartialMessage> (Promise.resolve(...))
alt resolution succeeds
Resolver-->>Ephemeral: PartialMessage { chat.id, message_id }
Ephemeral->>API: schedule deleteMessage(chat.id, message_id) after timeout
API-->>Ephemeral: delete result (errors ignored)
else resolution fails
Resolver-->>Ephemeral: null
Ephemeral-->>Handler: return early (no delete scheduled)
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/index.ts (1)
42-49:⚠️ Potential issue | 🟠 MajorFire-and-forget here can cause unobserved promise rejections.
At Line 42 and Line 58,
context.reply(...)is passed directly toephemeral(...)and discarded withvoid. Ifcontext.replyrejects,ephemeralrejects before its internal.catch(() => {}), so the rejection is not handled.🔧 Proposed hardening in
src/utils/messages.ts(single fix for all call sites)export async function ephemeral(message: MaybePromise<PartialMessage>, timeout = 20000): Promise<void> { - const msg = await Promise.resolve(message) - await wait(timeout) - .then(() => modules.shared.api.deleteMessage(msg.chat.id, msg.message_id)) - .catch(() => {}) + await Promise.resolve(message) + .then((msg) => wait(timeout).then(() => modules.shared.api.deleteMessage(msg.chat.id, msg.message_id))) + .catch(() => {}) }Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/index.ts` around lines 42 - 49, The fire-and-forget use of ephemeral(context.reply(fmt(...))) can lead to unobserved promise rejections if context.reply rejects; change call sites (the uses of ephemeral and context.reply in this file) to ensure the returned promise is handled—either await the ephemeral(...) call where possible or append a .catch(...) to it so rejections are observed. Specifically, update the calls that pass context.reply(...) directly into ephemeral (referencing the ephemeral function and context.reply + fmt invocation) so that context.reply failures are caught before being discarded (e.g., await ephemeral(...) or ephemeral(...).catch(() => {/*log or noop*/})).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/messages.ts`:
- Around line 50-54: The ephemeral function can throw before its internal .catch
because await Promise.resolve(message) is outside the promise chain; wrap the
entire body in a try-catch (or move the await inside the .then chain) so any
rejection from resolving message (e.g., context.reply()) is caught and ignored;
specifically update the ephemeral function to catch errors from await
Promise.resolve(message) and from modules.shared.api.deleteMessage(msg.chat.id,
msg.message_id) to prevent unhandled rejections when callers use void
ephemeral(...).
---
Outside diff comments:
In `@src/commands/index.ts`:
- Around line 42-49: The fire-and-forget use of
ephemeral(context.reply(fmt(...))) can lead to unobserved promise rejections if
context.reply rejects; change call sites (the uses of ephemeral and
context.reply in this file) to ensure the returned promise is handled—either
await the ephemeral(...) call where possible or append a .catch(...) to it so
rejections are observed. Specifically, update the calls that pass
context.reply(...) directly into ephemeral (referencing the ephemeral function
and context.reply + fmt invocation) so that context.reply failures are caught
before being discarded (e.g., await ephemeral(...) or ephemeral(...).catch(() =>
{/*log or noop*/})).
🪄 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: CHILL
Plan: Pro
Run ID: d0d03722-9000-4cc9-8f58-89e6319f1725
📒 Files selected for processing (12)
src/commands/index.tssrc/commands/invite.tssrc/commands/link-admin-dashboard.tssrc/commands/moderation/ban.tssrc/commands/moderation/del.tssrc/commands/moderation/kick.tssrc/commands/moderation/mute.tssrc/commands/pin.tssrc/lib/managed-commands/index.tssrc/utils/messages.tssrc/utils/types.tssrc/utils/users.ts
💤 Files with no reviewable changes (1)
- src/utils/users.ts
No description provided.