Refactor exception message handling with custom exception classes

[coderabbitai]

Background

In PR #323, CodeRabbit suggested refactoring exception message handling by introducing custom exception classes to address Ruff TRY003 warnings about long messages outside exception classes.

Current Approach

Currently, the codebase uses direct exception raising with detailed error messages that include CWE references, such as:

except ImportError as baton:
    raise ImportError("[CWE-440] Unable to import sys module.") from baton

Proposed Solution

Implement custom exception classes to encapsulate these messages for better maintainability:

class ImportFailureError(ImportError):
    """Custom exception for import failures with CWE references."""
    def __init__(self, component, cwe="440", *args, **kwargs):
        self.cwe = cwe
        message = f"[CWE-{cwe}] Unable to import {component}."
        super().__init__(message, *args, **kwargs)

# Usage example:
except ImportError as baton:
    raise ImportFailureError("sys module", "440") from baton

Benefits

1. Improved code maintainability
2. Consistent error messaging
3. Addresses Ruff TRY003 linting warnings
4. Better encapsulation of error information

References

• PR Additional AST-GREP rule for simplifying imports #323: Additional AST-GREP rule for simplifying imports #323
• Discussion: Additional AST-GREP rule for simplifying imports #323 (comment)

[reactive-firewall]

@coderabbitai I'm thinking that it would make sense to subclass ModuleNotFoundError instead as our errors are concerned with the result of sys.modules being effectively None for the given component

So something like:

class ImportFailureError(ModuleNotFoundError):
    """Custom exception for import failures with CWE references."""
    def __init__(self, component, cwe="440", *args, **kwargs) -> None:
        self.cwe = cwe
        message = f"[CWE-{cwe}] Unable to import {component}."
        super().__init__(message, *args, **kwargs)

# Usage example:
except ImportError as baton:
    raise ImportFailureError("sys module", "440") from baton

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for any existing implementation of ImportFailureError
rg "class ImportFailureError" -A 10

# Look for any error classes related to imports
rg "class.*Error.*import|import.*Error" --type py

# Find where ImportError is being caught
rg "except ImportError" --type py -A 3 -B 3

# Check issue #324 details
gh issue view 324

Length of output: 10161

---

🏁 Script executed:

#!/bin/bash
# Check if there's an exceptions.py file
fd "exceptions.py" --type f

# Look at the content of any exceptions.py file
cat multicast/exceptions.py 2>/dev/null || echo "File not found"

# Look for existing exception class definitions in the codebase
rg "class.*Error" --type py -l | xargs cat 2>/dev/null

Length of output: 47021

---

🏁 Script executed:

#!/bin/bash
# Find all instances of raising ImportError with CWE references
rg "raise ImportError.*CWE" -B2 -A1

# Check existing mapping for ModuleNotFoundError in EXIT_CODES
rg "ModuleNotFoundError" multicast/exceptions.py

Length of output: 9265

---

@reactive-firewall I agree that subclassing `ModuleNotFoundError` makes perfect sense for this use case! Since these exceptions specifically deal with modules not being found in `sys.modules`, the more specific exception type is semantically appropriate.

Based on the existing exception patterns in your codebase, here's a more complete implementation of the ImportFailureError class:

class ImportFailureError(ModuleNotFoundError):
    """
    Custom exception for import failures with CWE references.
    
    This exception is raised when a module cannot be imported,
    particularly when it's not available in sys.modules.
    
    Attributes:
        message (str): Description of the error.
        cwe (str): Common Weakness Enumeration reference code.
        component (str): The name of the component that failed to import.
        exit_code (int): The exit code associated with the error (127 by default).
    
    Testing:
        Testcase 1: Basic initialization. >>> error = ImportFailureError("sys module")
            >>> isinstance(error, ModuleNotFoundError)
            True
            >>> error.message
            '[CWE-440] Unable to import sys module.'
            >>> error.cwe
            '440'
            >>> error.component
            'sys module'
            >>> error.exit_code
            127
            
        Testcase 2: Custom CWE code.
            >>> error = ImportFailureError("socket", "758")
            >>> error.message
            '[CWE-758] Unable to import socket.'
            >>> error.cwe
            '758'
    """
    
    __module__ = "multicast.exceptions"
    
    __name__ = "multicast.exceptions.ImportFailureError"
    
    def __init__(self, component, cwe="440", *args, **kwargs) -> None:
        """
        Initialize ImportFailureError with component name and CWE reference.
        
        Parameters:
            component (str): The name of the component that failed to import.
            cwe (str): Common Weakness Enumeration reference code.
            *args: Variable length argument list.
            **kwargs: Arbitrary keyword arguments.
            
        Raises:
            TypeError: If component is not a string.
            
        Testing:
            Testcase 1: Initialize with cause exception.
                >>> original = ImportError("Original import error")
                >>> error = ImportFailureError("test module", "440", original)
                >>> error.__cause__ == original
                True
                
            Testcase 2: Initialize with custom message format.
                >>> error = ImportFailureError("custom module", "758", 
                ...                           message_template="Cannot load {component}: [CWE-{cwe}]")
                >>> error.message
                'Cannot load custom module: [CWE-758]'
        """
        if len(args) > 0 and isinstance(args[0], BaseException):
            cause = args[0]
            args = args[1:]
        else:
            cause = kwargs.pop("__cause__", None)
            
        if not isinstance(component, str):
            raise TypeError("[CWE-573] component must be a string")
            
        message_template = kwargs.pop("message_template", "[CWE-{cwe}] Unable to import {component}.")
        message = message_template.format(component=component, cwe=cwe)
        
        super().__init__(message, *args, **kwargs)
        
        if cause is not None:
            self.__cause__ = cause
            
        self.message = message
        self.cwe = cwe
        self.component = component
        self.exit_code = 127  # Match ModuleNotFoundError in EXIT_CODES

To use this new exception class:

try:
    from . import sys
except ImportError as baton:
    raise ImportFailureError("sys module", "440") from baton

This approach will provide consistent error messages, make the code more maintainable, and address those Ruff TRY003 warnings while following your codebase's patterns and conventions.

---

🧠 Learnings used

Learnt from: reactive-firewall
PR: reactive-firewall/ymmv#9
File: payload/bin/lsmap:17-27
Timestamp: 2024-11-07T04:15:11.692Z
Learning: Reactive-firewall prefers scripts to support both BSD and GNU versions of commands, with a slight preference for BSD.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#115
File: multicast/hear.py:180-180
Timestamp: 2024-09-19T03:43:24.037Z
Learning: Reactive-firewall prefers to re-use already loaded modules to keep memory overhead low when possible.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#115
File: multicast/hear.py:180-180
Timestamp: 2024-10-08T21:59:30.537Z
Learning: Reactive-firewall prefers to re-use already loaded modules to keep memory overhead low when possible.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T19:13:01.630Z
Learning: In the multicast project, when refactoring shell scripts, prefer improving organization within a single file over splitting into multiple files, unless the benefits of modularity clearly outweigh the added complexity in security, permissions, and debugging.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T19:12:21.246Z
Learning: For future scripts, reactive-firewall prefers to use only POSIX built-ins (like `printf` instead of `echo`) to align with CEP-8 conventions.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T19:12:14.590Z
Learning: Reactive-firewall prefers to specify the full raw URL when fetching files from gists to ensure correct retrieval of content, particularly when the file is in markdown format.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T18:53:29.833Z
Learning: For future scripts, reactive-firewall prefers to keep the shebang line with an extra space (e.g., `#! /bin/bash`) as per project style.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T19:12:34.497Z
Learning: For future reviews, remember that when drafting or updating CEP (Convention Enhancement Proposal) documents, it's important to adhere to the project's standard format and include all relevant sections such as Motivation, Detailed Explanation, Implementation Guidelines, Impact, and References.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: LICENSE.md:36-43
Timestamp: 2025-03-12T19:12:34.497Z
Learning: Reactive-firewall prefers that I verify URLs before suggesting changes, and avoid raising hypothetical issues without verification.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#235
File: .github/workflows/makefile-lint.yml:41-52
Timestamp: 2025-03-12T19:13:01.630Z
Learning: Reactive-firewall prefers to suppress errors in `apt-get install` commands when individual checks of the installed packages are performed afterwards, to allow for more granular error handling.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T19:12:21.246Z
Learning: In the multicast project's shell scripts, prefer organizing code into logical sections with clear comments (e.g., "=== Environment Setup ===") to maintain readability and maintainability while keeping related functions together.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2025-03-12T19:12:14.590Z
Learning: Reactive-firewall prefers that overviews and release notes include a mention of how strategies learned from the project have been applied to improve the final result.

---

Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: tests/check_pip:259-265
Timestamp: 2025-03-12T19:12:21.246Z
Learning: Reactive-firewall prefers that code changes adhere to SOLID design principles, specifically avoiding unnecessary complexity (AHA principle), maintaining the Single Responsibility Principle, and ensuring complex work is done on separate development branches (Open/Closed Principle).