Skip to content

Refactor Error Handling: Replace widespread error_exit() calls with Custom Exceptions #849

@Masterix0

Description

@Masterix0

Is your feature request related to a problem? Please describe.

Currently, the node-cli codebase extensively uses the error_exit() function (imported from node_cli.utils.helper) to handle error conditions and terminate the program execution with specific exit codes.
This pattern is found across various modules (e.g., core, operations, configs).

While error_exit() provides a way to exit with a specific code, this approach has several drawbacks:

  • Decentralized Control Flow: Errors cause immediate termination deep within function calls, making it harder to understand and manage the program's exit points.
  • Limited Error Information: Exceptions can carry more context and structured information about the error than simple exit codes and messages.
  • Reduced Flexibility: It prevents higher-level code from potentially catching specific errors to perform cleanup, logging, or alternative actions before exiting.
  • Testing Complexity: Testing functions that call error_exit() often requires mocking sys.exit, which can be less straightforward than asserting that specific exceptions are raised.
  • Deviation from Pythonic Practices: Standard Python error handling relies on raising and catching exceptions.

This issue was highlighted during a recent Pull Request review (PR #839) with the comment: "It is cleaner to handle base Exception class in one place. It is already handled in main.py. Suggest to raise custom exception type instead and run error_exit in main.py with specific error index."

Describe the solution you'd like

We propose refactoring the codebase to adopt a more standard exception-based error handling mechanism:

  1. Define Custom Exception Classes: Create a set of custom exception classes, potentially inheriting from a common base exception (e.g., NodeCliError). These could reside in a new node_cli/exceptions.py module. Examples might include ConfigurationError, DockerOperationError, NodeStateError, NetworkError, etc.
  2. Replace error_exit() with raise: Identify instances where error_exit() is called due to a specific error condition. Replace these calls with raise CustomException(...), passing relevant context if possible. Allow standard exceptions (like PermissionError, FileNotFoundError) to propagate where appropriate instead of catching them and calling error_exit.
  3. Centralize Exception Handling: Enhance the main try...except block in node_cli/main.py.
    • Add specific except clauses for the newly defined custom exceptions (or their base class) and potentially common standard exceptions if specific exit codes are desired for them.
    • Inside these handlers, map the caught exception type to the appropriate exit code (from node_cli.utils.exit_codes).
    • Call the error_exit() function only from within these central handlers in main.py to perform the final exit operation with the correct code and message.
    • Maintain the generic except Exception: handler to catch unexpected errors, ensuring it logs sufficient details (like the traceback) before exiting.
  4. Update Unit Tests: Modify existing tests that currently assert SystemExit based on error_exit calls (e.g., using pytest.raises(SystemExit) and checking excinfo.value.code). These tests must be updated to expect the new custom exceptions or relevant standard exceptions (like PermissionError, FileNotFoundError, etc.) where appropriate (e.g., using pytest.raises(SpecificException)).

This approach will centralize program termination logic, make error handling more explicit and flexible, and improve code maintainability and testability.

Describe alternatives you've considered

  • Maintaining Status Quo: Continue using error_exit() extensively. This perpetuates the issues mentioned above and requires specific handling in tests.
  • Returning Error Codes/Objects: Functions could return status indicators or objects instead of raising exceptions. This is generally less Pythonic for handling exceptional situations and can lead to verbose checking of return values throughout the code.

The proposed custom exception approach aligns best with standard practices and offers the most benefits for code clarity and robustness.

Additional context

This is a significant but valuable refactoring effort that will improve the overall quality and maintainability of the node-cli codebase.
It standardizes how errors are signaled and handled, making the application easier to debug and extend in the future.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions