Skip to content

Conversation

@cidrblock
Copy link
Contributor

@cidrblock cidrblock commented Aug 6, 2025

Overview

Replace direct util.sysexit() calls with a new ImmediateExit exception to provide clean, testable, and centralized immediate program termination.

Problem Solved

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:

except ScenarioFailureError as exc:
    util.sysexit(code=exc.code)  # Direct exit, hard to test

Solution

New ImmediateExit Exception:

class ImmediateExit(Exception):  # noqa: N818
    def __init__(self, message: str, code: int) -> None:
        super().__init__(message)
        self.message = message
        self.code = code

New Pattern:

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.

Key Improvements

  • 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

Implementation Details

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

Testing

  • ✅ 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

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.

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
@cidrblock cidrblock changed the title fix: Handle ScenarioFailureError gracefully when running all scenarios refactor: simplify exception handling with single ImmediateExit exception Aug 6, 2025
@cidrblock cidrblock changed the title refactor: simplify exception handling with single ImmediateExit exception refactor: introduce ImmediateExit exception for clean error handling Aug 6, 2025
@cidrblock cidrblock merged commit a7f9f2d into ansible:main Aug 6, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from Review to Done in 🧰 devtools project board 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

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants