Skip to content

fix: retry init requests and guard JSON parsing - #584

Open
sadsfae wants to merge 1 commit into
developmentfrom
fix/569-retry-init-json
Open

sadsfae wants to merge 1 commit into
developmentfrom
fix/569-retry-init-json

Conversation

@sadsfae

@sadsfae sadsfae commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #569

Summary

Two robustness issues in the init/read-only path:

  1. --retries never covered init requests. HTTPClient stored self.retries but get_raw performed a single session.get with no retry loop, so transient failures during the initial requests (session discovery, authentication, resource enumeration) failed immediately instead of retrying.

  2. json.loads sites lacked ValueError handling. A malformed response surfaced as a raw ValueError traceback instead of a clear BadfishException.

Changes

src/badfish/helpers/http_client.py

  • get_raw now wraps the session.get+read in a retry loop honoring self.retries:
    • Retries on transient failures (aiohttp.ClientError, TimeoutError, OSError) with a 1s pause between attempts.
    • Non-retryable: SSL certificate errors stay on the existing _handle_ssl_error path.
    • _continue=True still returns None on failure; final failure raises BadfishException("Failed to communicate with server.").
    • post_request / patch_request / delete_request semantics unchanged.
  • Added load_json(raw, context="response") helper that raises BadfishException("Error reading <context> from host.") on ValueError/TypeError.
  • Wrapped the unguarded json.loads in find_session_uri.

src/badfish/main.py

  • Imported load_json and replaced the unwrapped json.loads(raw.strip()) sites with it (sites already in try/except were left as-is):
Context Function
boot order get_boot
reset types get_reset_types
session find_session_uri
managers find_managers_resource (2 sites)
power state power state retrieval
job status check_schedule_job_status, check_job_status
firmware inventory firmware inventory
export job export job (2 sites)
import job import job

Test evidence

  • tests/test_http_client.py: 39 passed (including new test_get_raw_retries_then_succeeds, test_get_raw_retries_exhausted_raises, test_load_json_valid_returns_parsed, test_load_json_raises_on_invalid_json).
  • Targeted main.py-related suites (vendor detection, power, job queue, firmware inventory, SCP, change boot, boot-to, main coverage): all passed.

get_raw now honors self.retries by wrapping the session.get call in a
retry loop that retries transient failures (aiohttp.ClientError,
TimeoutError, OSError) with a short pause, while preserving the SSL
certificate error path and _continue=None-on-failure behavior.

Added a load_json() helper that raises BadfishException on malformed
JSON and replaced the unwrapped json.loads sites in helpers/http_client.py
and main.py with it, so a malformed response surfaces as a clear
"Error reading <context> from host." error instead of a raw ValueError.

fixes: #569
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/badfish/helpers/http_client.py 93.75% 2 Missing ⚠️
Flag Coverage Δ
unittests 94.84% <95.55%> (-0.04%) ⬇️

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.33% <100.00%> (ø)
src/badfish/helpers/http_client.py 95.95% <93.75%> (-0.77%) ⬇️
🚀 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