Skip to content

Fix Duco diagnostics crash on connection error#169322

Merged
erwindouna merged 3 commits into
home-assistant:devfrom
ronaldvdmeer:fix/duco-diagnostics-connection-error
Apr 28, 2026
Merged

Fix Duco diagnostics crash on connection error#169322
erwindouna merged 3 commits into
home-assistant:devfrom
ronaldvdmeer:fix/duco-diagnostics-connection-error

Conversation

@ronaldvdmeer
Copy link
Copy Markdown
Contributor

@ronaldvdmeer ronaldvdmeer commented Apr 27, 2026

Proposed change

The Duco diagnostics platform crashed with an unhandled DucoConnectionError when the device was unreachable during a diagnostics download. The exception propagated all the way up instead of returning a meaningful error to the user.

This PR wraps each API call in async_get_config_entry_diagnostics with a try/except that catches DucoConnectionError and raises HomeAssistantError, which Home Assistant handles gracefully.

Important

This fixes a crash in the diagnostics platform when the device is temporarily offline.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies a diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

@ronaldvdmeer ronaldvdmeer force-pushed the fix/duco-diagnostics-connection-error branch from b6ce185 to 32d8b37 Compare April 27, 2026 18:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Duco diagnostics downloads failing with an unhandled connection exception when the device is offline, by converting connection failures into a Home Assistant–native exception and adding a regression test.

Changes:

  • Catch DucoConnectionError during diagnostics retrieval and raise HomeAssistantError instead.
  • Add a translatable exception message for diagnostics connection failures.
  • Add a test that verifies a diagnostics request returns an HTTP 500 when any of the underlying diagnostics API calls fails with a connection error.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
homeassistant/components/duco/diagnostics.py Adds connection-error handling around diagnostics API calls.
homeassistant/components/duco/strings.json Adds a new translated exception message for diagnostics connection failures.
tests/components/duco/test_diagnostics.py Adds a regression test for connection errors during diagnostics download.

Comment thread homeassistant/components/duco/diagnostics.py
Comment thread homeassistant/components/duco/diagnostics.py
@ronaldvdmeer ronaldvdmeer marked this pull request as ready for review April 27, 2026 18:16
Copy link
Copy Markdown
Member

@erwindouna erwindouna left a comment

Choose a reason for hiding this comment

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

Good fix. Just a few points for discussion or patching. :)

Comment thread homeassistant/components/duco/strings.json Outdated
Comment thread tests/components/duco/test_diagnostics.py
@home-assistant home-assistant Bot marked this pull request as draft April 27, 2026 19:32
@home-assistant
Copy link
Copy Markdown
Contributor

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@ronaldvdmeer ronaldvdmeer marked this pull request as ready for review April 27, 2026 20:40
Copilot AI review requested due to automatic review settings April 27, 2026 20:40
@home-assistant home-assistant Bot requested a review from erwindouna April 27, 2026 20:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread homeassistant/components/duco/diagnostics.py
Comment thread tests/components/duco/test_diagnostics.py
Comment thread tests/components/duco/test_diagnostics.py
Copy link
Copy Markdown
Member

@erwindouna erwindouna 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 to me, thanks @ronaldvdmeer!

@erwindouna erwindouna merged commit fa0cf37 into home-assistant:dev Apr 28, 2026
33 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants