-
Notifications
You must be signed in to change notification settings - Fork 665
refactor: introduce ImmediateExit exception for clean error handling #4501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When running 'molecule test' without specifying scenarios (i.e., testing all scenarios) and platforms are missing from molecule.yml, the code would raise an unhandled ScenarioFailureError exception causing an ugly traceback. This fix wraps the get_configs() call in the 'run all scenarios' path with the same try-catch pattern already used elsewhere in the same function for specific scenario names. The fix ensures consistent error handling and provides a clean exit with proper error code instead of a traceback. Fixes: Instances missing from the 'platform' section traceback
…tion Replace complex three-exception hierarchy (ImmediateExitError, ImmediateExitCleanError, ImmediateExitFailError) with single ImmediateExit exception that requires explicit exit code. This design mimics SystemExit semantics while maintaining Exception inheritance for better framework integration. The caller must provide both message and exit code, making intent explicit and eliminating the need for separate success/failure classes. Key improvements: - 75% less exception code (1 class vs 3) - SystemExit-like API: ImmediateExit(message, code) - Centralized handling in click_command_ex decorator - Debug-aware logging (full traceback in debug mode for failures) - Cleaner imports and simpler testing Exception handling flow: - Code raises ImmediateExit with specific message and exit code - click_command_ex decorator catches and handles logging/termination - Success (code=0): info logging, failure (code!=0): error/exception logging - All paths call util.sysexit(code) for consistent termination Files changed: - Core: exceptions.py (new single exception), click_cfg.py (unified handler) - Conversions: command/base.py, dependency/base.py, provisioner/ansible.py, verifier/testinfra.py - Tests: Updated all exception expectations from old types to ImmediateExit - Added comprehensive test coverage for new exception and decorator behavior
alisonlhart
approved these changes
Aug 6, 2025
cidrblock
added a commit
to cidrblock/molecule
that referenced
this pull request
Aug 6, 2025
…nsible#4501) Replace direct `util.sysexit()` calls with a new `ImmediateExit` exception to provide clean, testable, and centralized immediate program termination. Previously, error conditions throughout Molecule called `util.sysexit(code)` directly, which: - Made testing difficult (hard to mock sys.exit calls consistently) - Scattered termination logic across multiple modules - Bypassed normal exception handling patterns - Provided inconsistent logging behavior - Could result in ugly unhandled tracebacks in some edge cases **Example of the original problem:** When running `molecule test` with missing platforms, users would see an ugly traceback instead of a clean error: ``` INFO default ➜ discovery: scenario test matrix: create, destroy CRITICAL Instances missing from the 'platform' section of molecule.yml. DETAILS Traceback (most recent call last): File "/home/user/.venv/bin/molecule", line 10, in <module> sys.exit(main()) ~~~~^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1442, in __call__ return self.main(*args, **kwargs) ~~~~~~~~~^^^^^^^^^^^^^^^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1363, in main rv = self.invoke(ctx) File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1830, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1226, in invoke return ctx.invoke(self.callback, **ctx.params) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/decorators.py", line 34, in new_func return f(get_current_context(), *args, **kwargs) File "/home/user/molecule/src/molecule/click_cfg.py", line 428, in wrapper return func(ctx) File "/home/user/molecule/src/molecule/command/test.py", line 82, in test base.execute_cmdline_scenarios(scenario_name, args, command_args, ansible_args, exclude) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 169, in execute_cmdline_scenarios _run_scenarios(scenarios, command_args, default_config) ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 225, in _run_scenarios create_results = execute_subcommand_default(default_config, "create") File "/home/user/molecule/src/molecule/command/base.py", line 308, in execute_subcommand_default execute_subcommand(default_config, subcommand) ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 344, in execute_subcommand return command(current_config).execute(args) ~~~~~~~^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 76, in __init__ self._setup() ~~~~~~~~~~~^^ File "/home/user/molecule/src/molecule/command/base.py", line 110, in _setup self._config.provisioner.manage_inventory() File "/home/user/molecule/src/molecule/provisioner/ansible.py", line 912, in _verify_inventory raise MoleculeError(msg) ~~~~~~~~~~~~~~~~~~~~~~~~ molecule.exceptions.MoleculeError: Instances missing from the 'platform' section of molecule.yml. ``` **Old pattern:** ```python except ScenarioFailureError as exc: util.sysexit(code=exc.code) # Direct exit, hard to test ``` **New ImmediateExit Exception:** ```python class ImmediateExit(Exception): # noqa: N818 def __init__(self, message: str, code: int) -> None: super().__init__(message) self.message = message self.code = code ``` **New Pattern:** ```python except ScenarioFailureError as exc: raise ImmediateExit("Scenario execution failed", code=exc.code) from exc ``` **Centralized Handling:** The `click_command_ex` decorator catches all `ImmediateExit` exceptions and provides: - Debug-aware logging (full traceback in debug mode, clean messages otherwise) - Consistent termination via `util.sysexit(code)` - Single point of control for all immediate exits **Result:** Now users see clean error messages instead of tracebacks: ``` INFO default ➜ discovery: scenario test matrix: create, destroy ERROR Instances missing from the 'platform' section of molecule.yml. ``` - **Better testability:** Exceptions can be caught and validated in tests - **Centralized termination:** All immediate exits go through one handler - **Consistent logging:** Debug mode shows full tracebacks, normal mode shows clean messages - **Exception semantics:** Proper exception hierarchy allows standard error handling - **Clear intent:** Explicit message and exit code requirements - **Clean user experience:** No more ugly tracebacks for expected error conditions **Exception Flow:** 1. Error conditions raise `ImmediateExit(message, code)` 2. `click_command_ex` decorator catches the exception 3. Conditional logging based on exit code and debug mode: - Success (code=0): `logger.info(message)` - Failure + debug: `logger.exception(message)` (full traceback) - Failure + normal: `logger.error(message)` (clean message) 4. `util.sysexit(code)` for program termination **Files Changed:** *Core Exception System:* - `src/molecule/exceptions.py` - New ImmediateExit exception class - `src/molecule/click_cfg.py` - Centralized exception handler in decorator *Conversion from util.sysexit to ImmediateExit:* - `src/molecule/command/base.py` - ScenarioFailureError handling - `src/molecule/dependency/base.py` - Dependency installation failures - `src/molecule/provisioner/ansible.py` - Missing platform validation - `src/molecule/verifier/testinfra.py` - Test verification failures *Updated Tests:* - `tests/unit/test_exceptions.py` - New comprehensive exception tests - `tests/unit/test_click_command_ex.py` - New decorator behavior tests - `tests/unit/command/test_base.py` - Updated to expect ImmediateExit - `tests/unit/provisioner/test_ansible.py` - Updated inventory validation tests - **✅ All tests pass:** 678 passed, 6 skipped, 0 failed - **✅ Pre-commit clean:** All quality checks passing - **✅ Coverage maintained:** 91% overall coverage - **✅ No regressions:** All existing functionality preserved This is an internal refactoring of error handling mechanisms. The public API, CLI behavior, and exit codes remain unchanged. Users will see cleaner error messages instead of tracebacks for expected error conditions. Provides a cleaner, more testable approach to immediate program termination while maintaining the same user-visible behavior. The exception-based approach allows for proper testing of error conditions and centralizes termination logic for better maintainability.
cidrblock
added a commit
to cidrblock/molecule
that referenced
this pull request
Aug 6, 2025
…nsible#4501) Replace direct `util.sysexit()` calls with a new `ImmediateExit` exception to provide clean, testable, and centralized immediate program termination. Previously, error conditions throughout Molecule called `util.sysexit(code)` directly, which: - Made testing difficult (hard to mock sys.exit calls consistently) - Scattered termination logic across multiple modules - Bypassed normal exception handling patterns - Provided inconsistent logging behavior - Could result in ugly unhandled tracebacks in some edge cases **Example of the original problem:** When running `molecule test` with missing platforms, users would see an ugly traceback instead of a clean error: ``` INFO default ➜ discovery: scenario test matrix: create, destroy CRITICAL Instances missing from the 'platform' section of molecule.yml. DETAILS Traceback (most recent call last): File "/home/user/.venv/bin/molecule", line 10, in <module> sys.exit(main()) ~~~~^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1442, in __call__ return self.main(*args, **kwargs) ~~~~~~~~~^^^^^^^^^^^^^^^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1363, in main rv = self.invoke(ctx) File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1830, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/core.py", line 1226, in invoke return ctx.invoke(self.callback, **ctx.params) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/.venv/lib64/python3.13/site-packages/click/decorators.py", line 34, in new_func return f(get_current_context(), *args, **kwargs) File "/home/user/molecule/src/molecule/click_cfg.py", line 428, in wrapper return func(ctx) File "/home/user/molecule/src/molecule/command/test.py", line 82, in test base.execute_cmdline_scenarios(scenario_name, args, command_args, ansible_args, exclude) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 169, in execute_cmdline_scenarios _run_scenarios(scenarios, command_args, default_config) ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 225, in _run_scenarios create_results = execute_subcommand_default(default_config, "create") File "/home/user/molecule/src/molecule/command/base.py", line 308, in execute_subcommand_default execute_subcommand(default_config, subcommand) ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 344, in execute_subcommand return command(current_config).execute(args) ~~~~~~~^^^^^^^^^^^^^^^^ File "/home/user/molecule/src/molecule/command/base.py", line 76, in __init__ self._setup() ~~~~~~~~~~~^^ File "/home/user/molecule/src/molecule/command/base.py", line 110, in _setup self._config.provisioner.manage_inventory() File "/home/user/molecule/src/molecule/provisioner/ansible.py", line 912, in _verify_inventory raise MoleculeError(msg) ~~~~~~~~~~~~~~~~~~~~~~~~ molecule.exceptions.MoleculeError: Instances missing from the 'platform' section of molecule.yml. ``` **Old pattern:** ```python except ScenarioFailureError as exc: util.sysexit(code=exc.code) # Direct exit, hard to test ``` **New ImmediateExit Exception:** ```python class ImmediateExit(Exception): # noqa: N818 def __init__(self, message: str, code: int) -> None: super().__init__(message) self.message = message self.code = code ``` **New Pattern:** ```python except ScenarioFailureError as exc: raise ImmediateExit("Scenario execution failed", code=exc.code) from exc ``` **Centralized Handling:** The `click_command_ex` decorator catches all `ImmediateExit` exceptions and provides: - Debug-aware logging (full traceback in debug mode, clean messages otherwise) - Consistent termination via `util.sysexit(code)` - Single point of control for all immediate exits **Result:** Now users see clean error messages instead of tracebacks: ``` INFO default ➜ discovery: scenario test matrix: create, destroy ERROR Instances missing from the 'platform' section of molecule.yml. ``` - **Better testability:** Exceptions can be caught and validated in tests - **Centralized termination:** All immediate exits go through one handler - **Consistent logging:** Debug mode shows full tracebacks, normal mode shows clean messages - **Exception semantics:** Proper exception hierarchy allows standard error handling - **Clear intent:** Explicit message and exit code requirements - **Clean user experience:** No more ugly tracebacks for expected error conditions **Exception Flow:** 1. Error conditions raise `ImmediateExit(message, code)` 2. `click_command_ex` decorator catches the exception 3. Conditional logging based on exit code and debug mode: - Success (code=0): `logger.info(message)` - Failure + debug: `logger.exception(message)` (full traceback) - Failure + normal: `logger.error(message)` (clean message) 4. `util.sysexit(code)` for program termination **Files Changed:** *Core Exception System:* - `src/molecule/exceptions.py` - New ImmediateExit exception class - `src/molecule/click_cfg.py` - Centralized exception handler in decorator *Conversion from util.sysexit to ImmediateExit:* - `src/molecule/command/base.py` - ScenarioFailureError handling - `src/molecule/dependency/base.py` - Dependency installation failures - `src/molecule/provisioner/ansible.py` - Missing platform validation - `src/molecule/verifier/testinfra.py` - Test verification failures *Updated Tests:* - `tests/unit/test_exceptions.py` - New comprehensive exception tests - `tests/unit/test_click_command_ex.py` - New decorator behavior tests - `tests/unit/command/test_base.py` - Updated to expect ImmediateExit - `tests/unit/provisioner/test_ansible.py` - Updated inventory validation tests - **✅ All tests pass:** 678 passed, 6 skipped, 0 failed - **✅ Pre-commit clean:** All quality checks passing - **✅ Coverage maintained:** 91% overall coverage - **✅ No regressions:** All existing functionality preserved This is an internal refactoring of error handling mechanisms. The public API, CLI behavior, and exit codes remain unchanged. Users will see cleaner error messages instead of tracebacks for expected error conditions. Provides a cleaner, more testable approach to immediate program termination while maintaining the same user-visible behavior. The exception-based approach allows for proper testing of error conditions and centralizes termination logic for better maintainability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Replace direct
util.sysexit()calls with a newImmediateExitexception to provide clean, testable, and centralized immediate program termination.Problem Solved
Previously, error conditions throughout Molecule called
util.sysexit(code)directly, which:Example of the original problem:
When running
molecule testwith missing platforms, users would see an ugly traceback instead of a clean error:Old pattern:
Solution
New ImmediateExit Exception:
New Pattern:
Centralized Handling:
The
click_command_exdecorator catches allImmediateExitexceptions and provides:util.sysexit(code)Result:
Now users see clean error messages instead of tracebacks:
Key Improvements
Implementation Details
Exception Flow:
ImmediateExit(message, code)click_command_exdecorator catches the exceptionlogger.info(message)logger.exception(message)(full traceback)logger.error(message)(clean message)util.sysexit(code)for program terminationFiles Changed:
Core Exception System:
src/molecule/exceptions.py- New ImmediateExit exception classsrc/molecule/click_cfg.py- Centralized exception handler in decoratorConversion from util.sysexit to ImmediateExit:
src/molecule/command/base.py- ScenarioFailureError handlingsrc/molecule/dependency/base.py- Dependency installation failuressrc/molecule/provisioner/ansible.py- Missing platform validationsrc/molecule/verifier/testinfra.py- Test verification failuresUpdated Tests:
tests/unit/test_exceptions.py- New comprehensive exception teststests/unit/test_click_command_ex.py- New decorator behavior teststests/unit/command/test_base.py- Updated to expect ImmediateExittests/unit/provisioner/test_ansible.py- Updated inventory validation testsTesting
Backwards Compatibility
This is an internal refactoring of error handling mechanisms. The public API, CLI behavior, and exit codes remain unchanged. Users will see cleaner error messages instead of tracebacks for expected error conditions.
Use Case
Provides a cleaner, more testable approach to immediate program termination while maintaining the same user-visible behavior. The exception-based approach allows for proper testing of error conditions and centralizes termination logic for better maintainability.