Conversation
1. Added _log_report_send_failure() helper method (~45 lines) - Logs failed report sends with full context (reporter, guild, channel, log channel, error) - Sends detailed notification to bot owners including the report content - Handles message truncation to fit within Discord's 2000 character limit - Includes proper error handling for owner notifications 2. Added _log_report_failure() helper method (~50 lines) - Logs failed reports from command error handlers - Extracts and logs report content when available - Sends comprehensive error notifications to bot owners - Handles cases where report content is unavailable 3. Updated on_cmd_report_error() method - Added report content extraction logic for unexpected errors - Attempts to extract from ctx.kwargs (for hybrid commands) or message content - Calls _log_report_failure() to preserve report content - Handles extraction failures gracefully 4. Updated on_cmd_emergency_error() method - Same enhancements as on_cmd_report_error() for emergency reports - Ensures emergency reports are never lost 5. Enhanced do_report() method - Wrapped log channel send operations in try-except block - Calls _log_report_send_failure() when sending fails - Re-raises exception so error handlers can process it - Preserves report content even when delivery fails
Matsukami7
added
enhancement
| except Exception as e: | ||
| logger.error(f"Failed to send error to owners: {e}") | ||
| # For hybrid commands, the message parameter is passed as a keyword argument | ||
| if hasattr(ctx, "kwargs") and "message" in ctx.kwargs: |
There was a problem hiding this comment.
Questions
- is this necessary? Is ctx ever not a guild ctx? Is specific handling needed here?
- why are there different control flows for the different types. Is the message content different?
| except Exception as e: | ||
| logger.error(f"Failed to send error to owners: {e}") | ||
| # For hybrid commands, the message parameter is passed as a keyword argument | ||
| if hasattr(ctx, "kwargs") and "message" in ctx.kwargs: |
There was a problem hiding this comment.
it's best to not directly check and add it to the try except
| await self._send_cmd_error_response(ctx, error, f"You are on cooldown. Try again in <t:{retry_timestamp}:R>") | ||
| return | ||
|
|
||
| # Unexpected error - extract and log report content |
There was a problem hiding this comment.
Same as lines 174-188. Should ideally be in a function or moved into _log_report_failure
| await self.do_report(ctx.channel, ctx.message, message, False, ctx.interaction) | ||
|
|
||
| @cmd_report.error | ||
| async def on_cmd_report_error(self, ctx: commands.GuildContext, error): |
There was a problem hiding this comment.
if this isnt always a guildContext this typehint is wrong.
relates to: https://github.com/rHomelab/LabBot-Cogs/pull/371/changes#r2765945867
| else: | ||
| # Not an emergency or not a text-like channel (maybe can't retrieve members), just send the embed | ||
| await log_channel.send(embed=embed) | ||
| try: |
There was a problem hiding this comment.
This is pointless. This error already propagates up and is caught by the error handlers already
| except discord.Forbidden: | ||
| logger.error("Failed to send config error notification to guild owner - no DM permissions") | ||
|
|
||
| async def _log_report_send_failure( # noqa: PLR0913 |
There was a problem hiding this comment.
This entire function is pointless.
See: https://github.com/rHomelab/LabBot-Cogs/pull/371/changes#r2766079399
| logger.error("Report content: <unavailable>") | ||
|
|
||
| # Prepare message for bot owners | ||
| base_msg = ( |
There was a problem hiding this comment.
This is a confusing implementation. Ideally construct the entire message then batch it out instead of crazy logic to truncate.
line 566-588
|
|
||
| # Send to bot owners | ||
| try: | ||
| await ctx.bot.send_to_owners(base_msg) |
There was a problem hiding this comment.
Ideally, this function should try to log to the log_channel first then fall back to sending to the owners
| report_body = report_body[:EMBED_FIELD_VALUE_LIMIT] | ||
| logger.info(f"Report content truncated from {original_length} to {len(report_body)} characters") | ||
|
|
||
| log_channel = await self.get_log_channel(channel.guild) |
There was a problem hiding this comment.
If no log channel is set, the report is also not logged. This should raise an error instead of returning
2 participants
Initial Checklist
Details
Does this resolve an issue?
Resolves #370
Changes
Features / Fixes
Breaking Changes
Additional
Any further notes or comments you want to make.