Skip to content

🔧 Improve exception handling to avoid broad exception catching #122

@anthropic-code-agent

Description

@anthropic-code-agent

Description

Multiple cogs in the repository use overly broad exception handlers (except Exception) that swallow all errors, making debugging difficult and potentially hiding bugs.

Locations

1. empty_voices/api.py - Multiple locations

Lines 74-78:

except Exception as e:
    log.error(f"Error cleaning up deleted channel from config: {e}")

Line 241 and others:

except Exception as e:
    log.error(f"Error: {e}")

Problem: These catch all exceptions including KeyboardInterrupt, SystemExit, and unexpected errors. They log the error message but not the full traceback, making debugging harder.

2. hat/hat.py - Bare exception handlers

Lines 114, 127-128:

except Exception:
    pass  # ❌ Silently swallows all errors

Problem: These completely silence all exceptions, including unexpected ones. No logging, no error reporting. If something goes wrong, there's no way to know.

3. nw_server_status/server_status.py - Background task

Lines 40-48:

@tasks.loop(minutes=5.0)
async def refresh_queue_data(self):
    try:
        # ...
    except Exception:
        logger.exception("Error in task")  # ← Better, but still too broad

Impact

  • Hard to debug: When errors are caught broadly, root causes are hidden
  • Masks bugs: Unexpected exceptions are swallowed instead of being raised
  • Silent failures: Some handlers use bare pass, giving no indication of problems
  • Missing context: Not using exc_info=True loses stack traces

Recommended Fixes

For empty_voices/api.py:

Replace:

except Exception as e:
    log.error(f"Error: {e}")

With specific exception types:

except (discord.HTTPException, discord.NotFound) as e:
    log.error(f"Discord error: {e}", exc_info=True)
except KeyError as e:
    log.error(f"Configuration error: {e}", exc_info=True)
except Exception as e:
    log.exception(f"Unexpected error: {e}")
    raise  # Re-raise unexpected exceptions

For hat/hat.py:

Replace:

except Exception:
    pass

With:

except (discord.NotFound, discord.Forbidden):
    pass  # Expected cases where message/file doesn't exist
except Exception as e:
    log.exception(f"Unexpected error in cleanup: {e}")

For nw_server_status/server_status.py:

Replace:

except Exception:
    logger.exception("Error in task")

With:

except (httpx.HTTPError, httpx.TimeoutException) as e:
    logger.error(f"API request failed: {e}", exc_info=True)
except Exception as e:
    logger.exception(f"Unexpected error in background task: {e}")
    # Optionally re-raise to stop task on critical errors

Best Practices

  1. Catch specific exceptions when possible (e.g., discord.NotFound, ValueError)
  2. Use log.exception() instead of log.error() for full tracebacks
  3. Don't silence unexpected exceptions - either log them or re-raise
  4. Document expected exceptions with comments
  5. Avoid bare except Exception: pass - always log something

Priority

MEDIUM - Affects debuggability and code quality but doesn't break functionality.

Testing

After fixing:

  1. Intentionally cause errors (e.g., invalid API calls, deleted channels)
  2. Verify errors are properly logged with full stack traces
  3. Verify expected errors (like discord.NotFound) are handled gracefully
  4. Check that unexpected errors are not silently swallowed

Related

This is part of a comprehensive code review. See other issues for additional improvements.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions