Skip to content

Add support for 500 and 429 status codes#620

Merged
agners merged 2 commits into
masterfrom
add-http-status-500
Feb 2, 2026
Merged

Add support for 500 and 429 status codes#620
agners merged 2 commits into
masterfrom
add-http-status-500

Conversation

@mdegat01
Copy link
Copy Markdown
Contributor

@mdegat01 mdegat01 commented Dec 10, 2025

Supervisor can now return 500 and 429 status codes under normal circumstances as of home-assistant/supervisor#6303 . This adds support for them to the CLI.

500 is extremely generic and is also returned in case of catastrophic failure by aiohttp. Therefore added support for a potential non-JSON response to handle that case somewhat sensibly.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling for rate-limited (429) responses to ensure they are processed as known JSON errors.
    • Enhanced reporting for internal server (500) errors with clearer "unknown error occurred" guidance and direction to check supervisor logs for diagnostics.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 10, 2025

📝 Walkthrough

Walkthrough

Adds handling for HTTP 429 (Too Many Requests) to JSON error processing and adds handling for HTTP 500 (Internal Server Error) to return a supervisor-log diagnostic when the response is not JSON. Also reorders imports (non-functional).

Changes

Cohort / File(s) Summary
Error handling improvements
client/client.go
Treats 429 as a JSON-known error; enforces JSON check for 500 and returns a supervisor-logs diagnostic message when non-JSON; minor import reordering.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Potential syntax error: Verify the HTTP 500 error message string is properly quoted to avoid a compile error.
  • Error message completeness: Confirm the supervisor logs message text is intentional and final.
  • Status code integration: Ensure new 429 and 500 branches integrate correctly with existing generic JSON error handling.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding support for HTTP 500 and 429 status codes in error handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-http-status-500

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 034b8b0 and 4f3ad6f.

📒 Files selected for processing (1)
  • client/client.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
*/**(html|markdown|md)

⚙️ CodeRabbit configuration file

*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.

  • Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
  • In step-by-step instructions, front the location phrase in the instructional sentence.
  • In step-by-step instructions, front the 'goal' in the instructional sentence.
  • In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
  • do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'

*/**(html|markdown|md): - Use bold to mark UI strings.

  • If "" are used to mark UI strings, replace them by bold.

  • Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"

  • Use sentence-style capitalization also in headings.

do not comment on HTML used for icons

Avoid flagging inline HTML for embedding videos in future reviews for this repository.

Files:

  • client/client.go
🔇 Additional comments (2)
client/client.go (2)

8-10: Import alias for logrus is appropriate

Using the log alias matches existing log.WithField usage and is idiomatic; no issues here.


24-28: Adding 429 to JSON-handled statuses is correct

Treating http.StatusTooManyRequests like the other JSON error statuses and enforcing a JSON Content-Type is consistent with the Supervisor behavior and the surrounding logic.

Comment thread client/client.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Member

@sairon sairon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, added the missing quote from the bot's suggestion.

@agners agners merged commit f79db56 into master Feb 2, 2026
9 checks passed
@agners agners deleted the add-http-status-500 branch February 2, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants