Skip to content

fix: propagate handler failures to CLI exit codes - #585

Open
sadsfae wants to merge 1 commit into
developmentfrom
fix/568-exit-code-propagation
Open

fix: propagate handler failures to CLI exit codes#585
sadsfae wants to merge 1 commit into
developmentfrom
fix/568-exit-code-propagation

Conversation

@sadsfae

@sadsfae sadsfae commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #568

Summary

execute_badfish() in src/badfish/main.py invoked most handlers with a bare await badfish.<handler>(), discarding their return value. When a handler returned False to signal a failed operation, result stayed True and the CLI exited 0 (success) even though the command actually failed.

This change captures each operation-failure handler's return value and, on False, sets result = False, which drives main() to return exit code 1. Success (returning True, or None in a few handlers) leaves result = True -> exit 0.

Wrapped handlers (False == operation failed)

  • check_schedule_job_status
  • reset_idrac
  • reset_bmc
  • get_power_consumed_watts
  • list_interfaces
  • list_gpu
  • mount_virtual_media
  • unmount_virtual_media
  • set_bios_password
  • remove_bios_password
  • get_scp_targets
  • export_scp
  • import_scp
  • get_nic_fqdds
  • get_nic_attribute / get_nic_attribute_info
  • set_nic_attribute

Informational-False handlers intentionally left untouched

A False return does not mean a failure for these, so they must continue to exit 0:

  • check_virtual_mediaFalse = no virtual media mounted (not an error)
  • check_remote_imageFalse = not attached / unsupported (informational)
  • list_job_queue
  • check_boot
  • take_screenshot — already raises BadfishException on failure
  • get_sriov_mode
  • power-state queries (get_power_state)

Design note

Failures are propagated via the result = False channel — the same channel the already-fixed boot_to path uses — rather than raising BadfishException. Every wrapped handler already logs its own specific error before returning False; raising an exception through the dispatch would emit a duplicate error line and break existing CLI-output assertions. result = False produces a single clean error message and still yields exit code 1.

Tests

Added tests/test_exit_code_propagation.py (35 cases) proving:

  • handler returns False -> execute_badfish returns ("host", False) -> CLI exit 1
  • handler returns True -> exit 0
  • handlers returning None on success (e.g. set_bios_password) are not misreported as failures
  • check_virtual_media / check_remote_image returning False are not treated as failures

Verified:

  • PYTHONPATH=src python -m pytest tests/test_exit_code_propagation.py -v -> 35 passed
  • PYTHONPATH=src python -m pytest tests/ -q -> 491 passed, 1 pre-existing unrelated failure (tests/test_progress.py::test_terminal_renders_progress_when_enabled)
  • The new failure-propagation tests fail on the pre-fix code (regression-valid).

execute_badfish() discarded the False return value from ~20 handler
calls, so the CLI exited 0 even when an operation failed. Capture each
operation-failure handler's return and set result=False (the channel
already used by boot_to), which drives main() to exit 1.

Wrapped handlers (False == operation failed): check_schedule_job_status,
reset_idrac, reset_bmc, get_power_consumed_watts, list_interfaces,
list_gpu, mount_virtual_media, unmount_virtual_media, set_bios_password,
remove_bios_password, get_scp_targets, export_scp, import_scp,
get_nic_fqdds, get_nic_attribute(_info), set_nic_attribute.

Informational-False handlers intentionally left untouched:
check_virtual_media (False = nothing mounted = success),
check_remote_image (False = not attached/unsupported), list_job_queue,
check_boot, take_screenshot (raises), get_sriov_mode, power-state queries.

Note: failures are routed through result=False rather than raising
BadfishException because every wrapped handler already logs its own
specific error before returning False; raising would duplicate the error
line and break existing CLI-output assertions.

fixes: #568
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 94.90% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/badfish/main.py 93.38% <100.00%> (+0.04%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant