Skip to content

Implement strike message failover#573

Merged
MattyTheHacker merged 4 commits intomainfrom
254-strike-message-channel-failover
Jul 9, 2025
Merged

Implement strike message failover#573
MattyTheHacker merged 4 commits intomainfrom
254-strike-message-channel-failover

Conversation

@MattyTheHacker
Copy link
Copy Markdown
Member

Adds error handling to DM messages and allows use of webhook if failure

@MattyTheHacker MattyTheHacker self-assigned this Jul 9, 2025
@MattyTheHacker MattyTheHacker added enhancement New feature or request sync Request bots to automatically keep this PR up to date with it's base branch labels Jul 9, 2025
@MattyTheHacker MattyTheHacker linked an issue Jul 9, 2025 that may be closed by this pull request
@MattyTheHacker MattyTheHacker enabled auto-merge (squash) July 9, 2025 10:06
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@MattyTheHacker MattyTheHacker force-pushed the 254-strike-message-channel-failover branch from 99f886b to f66692f Compare July 9, 2025 10:08
Copy link
Copy Markdown
Member

@CarrotManMatt CarrotManMatt left a comment

Choose a reason for hiding this comment

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

Please could utils/tex_bot.py line 507 also have a newline added after it.
I'd do it but then it would break the PR review.

Comment thread cogs/strike.py
Co-authored-by: Matt Norton <matt@carrotmanmatt.com>
Signed-off-by: Matty Widdop <18513864+MattyTheHacker@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances error handling for sending strike confirmation messages by adding failover logic when direct messages (DMs) are forbidden and formatting adjustments to notification messages.

  • Wraps DM channel creation and message sends in try/except discord.Forbidden, falling back to a log/webhook channel or aborting the operation.
  • Adjusts f-string formatting for clarity in user notifications.
  • Suppresses a linter warning (PLR0915) to allow a large handler function.
Comments suppressed due to low confidence (1)

cogs/strike.py:462

  • No tests cover the scenario where creating a DM channel raises discord.Forbidden and triggers the webhook fallback; consider adding unit tests or integration tests for this failure path to ensure the fallback works as intended.
            except discord.Forbidden:

Comment thread cogs/strike.py
Comment thread cogs/strike.py
Copy link
Copy Markdown
Member

@CarrotManMatt CarrotManMatt left a comment

Choose a reason for hiding this comment

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

Please could utils/tex_bot.py line 507 also have a newline added after it.
I'd do it but then it would break the PR review.

@MattyTheHacker
Copy link
Copy Markdown
Member Author

Please could utils/tex_bot.py line 507 also have a newline added after it.
I'd do it but then it would break the PR review.

out of scope babes xx

@CarrotManMatt
Copy link
Copy Markdown
Member

Nah nah akchually cos you added

return await self.bot.fetch_log_channel()
which references the function that needs fixing

Don't make me do a one line formatting PR

@MattyTheHacker MattyTheHacker merged commit 18e3a26 into main Jul 9, 2025
10 checks passed
@MattyTheHacker MattyTheHacker deleted the 254-strike-message-channel-failover branch July 9, 2025 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request sync Request bots to automatically keep this PR up to date with it's base branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve strike cog to failover to discord channel if DM fail

3 participants