Skip to content

fix: banall logs, forward exception, user storage trigger#131

Merged
toto04 merged 3 commits intomainfrom
fixes
Apr 23, 2026
Merged

fix: banall logs, forward exception, user storage trigger#131
toto04 merged 3 commits intomainfrom
fixes

Conversation

@lorenzocorallo
Copy link
Copy Markdown
Member

No description provided.

@lorenzocorallo lorenzocorallo requested a review from toto04 April 23, 2026 15:18
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

Walkthrough

The pull request extends user storage middleware to capture chat member joins, improves error detection in message forwarding with specific handling for invalid message IDs, removes redundant separator messages from the ban-all workflow, and adds message deletion before initiating ban operations.

Changes

Cohort / File(s) Summary
User Storage & Membership
src/middlewares/message-user-storage.ts
Added chat_member middleware to detect and upsert users when they join (status transition from left to member) into the in-memory user storage.
Logger & Ban Flow
src/modules/tg-logger/index.ts, src/modules/tg-logger/report.ts
Enhanced error handling in TgLogger.forward to specifically detect MESSAGE_ID_INVALID errors and emit targeted warnings; removed redundant separator message from banAll; added message deletion before initiating ban action.

Possibly related PRs

  • fix: log audit for banall #98: Modifies the BAN ALL flow and message deletion behavior, directly related to the changes made in the ban workflow and message handling in this PR.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses all three main changes: user storage trigger (chat_member middleware), forward exception handling (MESSAGE_ID_INVALID detection), and banall logs (deletion of separator message and reported message).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/modules/tg-logger/report.ts (1)

142-154: ⚠️ Potential issue | 🟠 Major

banAll(...) is no longer awaited — return value discarded and rejections become unhandled.

banAll returns Promise<string | null> and can perform multiple API calls (including this.log) that may reject. With the missing await, any rejection bubbles up as an unhandled promise rejection (which your own exception handler treats as UNHANDLED_PROMISE), and the returned string (e.g. the "ERROR" message when logging the banAll header fails) is silently dropped instead of being surfaced as callback feedback.

If the fire-and-forget is intentional (so editReportMessage runs immediately), at minimum attach a .catch to avoid unhandled rejections. Otherwise, await it and, ideally, surface the error string via the menu feedback.

Proposed fix
-        await ctx.api.deleteMessage(data.message.chat.id, data.message.message_id).catch(() => {})
-        modules
-          .get("tgLogger")
-          .banAll(
-            data.message.from,
-            ctx.from,
-            "BAN",
-            `Started after report by ${data.reporter.username ?? data.reporter.id}`
-          )
-        await editReportMessage(data, ctx, "🚨 Start BAN ALL")
-        return null
+        await ctx.api.deleteMessage(data.message.chat.id, data.message.message_id).catch(() => {})
+        void modules
+          .get("tgLogger")
+          .banAll(
+            data.message.from,
+            ctx.from,
+            "BAN",
+            `Started after report by ${data.reporter.username ?? data.reporter.id}`
+          )
+          .catch(() => {})
+        await editReportMessage(data, ctx, "🚨 Start BAN ALL")
+        return null
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/tg-logger/report.ts` around lines 142 - 154, The call to
modules.get("tgLogger").banAll in the cb handler is not awaited so its Promise
rejection and return value are lost; either await the result and surface its
returned string into the callback feedback or, if you truly want
fire-and-forget, attach a .catch to handle errors. Update the cb implementation
around modules.get("tgLogger").banAll to use either "await
modules.get('tgLogger').banAll(...)" and propagate/return the resulting string
into editReportMessage/menu feedback, or chain ".catch(err => { /* log via
this.log/exception handler or call editReportMessage with error */ })" to
prevent unhandled rejections; reference the cb function,
modules.get("tgLogger").banAll, and editReportMessage when making the change.
🤖 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/middlewares/message-user-storage.ts`:
- Around line 47-55: The current chat_member handler only caches users for old
status "left" → "member", so other join transitions (e.g., "kicked"→"member"
after unban or "restricted"→"member") are missed; update the filter used in
this.composer.on("chat_member") to detect a broader set of join
transitions—either by checking that ctx.chatMember.new_chat_member.status ===
"member" and ctx.chatMember.old_chat_member.status is not "member" (or belongs
to a set like {"left","kicked","restricted"})—then continue to call
this.userStorage.set(...) with ctx.chatMember.new_chat_member.user.id and the
user object as before; ensure you reference the same symbols (this.composer.on,
filter callback, ctx.chatMember.old_chat_member.status,
ctx.chatMember.new_chat_member.status, this.userStorage.set) so the change is
localized and safe.

---

Outside diff comments:
In `@src/modules/tg-logger/report.ts`:
- Around line 142-154: The call to modules.get("tgLogger").banAll in the cb
handler is not awaited so its Promise rejection and return value are lost;
either await the result and surface its returned string into the callback
feedback or, if you truly want fire-and-forget, attach a .catch to handle
errors. Update the cb implementation around modules.get("tgLogger").banAll to
use either "await modules.get('tgLogger').banAll(...)" and propagate/return the
resulting string into editReportMessage/menu feedback, or chain ".catch(err => {
/* log via this.log/exception handler or call editReportMessage with error */
})" to prevent unhandled rejections; reference the cb function,
modules.get("tgLogger").banAll, and editReportMessage when making the change.
🪄 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: 95b3cee2-1587-4d01-9686-e8dbfe74073e

📥 Commits

Reviewing files that changed from the base of the PR and between f132aeb and 3197bd7.

📒 Files selected for processing (3)
  • src/middlewares/message-user-storage.ts
  • src/modules/tg-logger/index.ts
  • src/modules/tg-logger/report.ts

Comment thread src/middlewares/message-user-storage.ts
@toto04 toto04 merged commit c2fbe51 into main Apr 23, 2026
2 checks passed
@toto04 toto04 deleted the fixes branch April 23, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants