Catch ClientConnectorError and TimeOutError in APSystems #132027

Thomas55555 · 2024-12-01T21:01:53Z

Breaking change

Proposed change

Use the same logic in the number platform, like in the switch platform:

core/homeassistant/components/apsystems/switch.py

Lines 42 to 48 in 98734eb

    
           try: 
        
               status = await self._api.get_device_power_status() 
        
           except (TimeoutError, ClientConnectionError, InverterReturnedError): 
        
               self._attr_available = False 
        
           else: 
        
               self._attr_available = True 
        
               self._attr_is_on = status

Benefits:

No uncaught errors
Entity doesn't stay avaibale all the time, even when it's unavailabe

Bugfix, please include in next beta/patch release.

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

This PR fixes or closes issue: fixes APSysttem integration - solar_max_output update fails with TimeoutError #121419
This PR is related to issue:
Link to documentation pull request:

Checklist

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.

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

Documentation added/updated for www.home-assistant.io

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 link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2024-12-01T21:02:00Z

Hey there @mawoka-myblock, @SonnenladenGmbH, mind taking a look at this pull request as it has been labeled with an integration (apsystems) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of apsystems can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign apsystems Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

epenet · 2024-12-02T09:52:23Z

homeassistant/components/apsystems/number.py

+        try:
+            status = await self._api.get_max_power()
+        except ClientConnectorError:
+            self._attr_available = False


See https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/log-when-unavailable

If internet/device/service is unavailable, log once when unavailable and once when back connected

It looks to me like you never log...

Edit: Or maybe it's because you don't want to log for each entity...

This is true, the integration never logs the change. How is it supposed to be? Would it be reasonable to log for every platform (switch, number and sensor)? As each of them uses a kind of different method regarding the data fetching

Standard way would be something like this:

Suggested change

self._attr_available = False

if self._attr_available:

_LOGGER.info(...offline)

self._attr_available = False

else:

if not self._attr_available:

_LOGGER.info(...back online)

self._attr_available = True

But I don't know enough about apsystems to make a proper recommendation.
We don't want to flood, but I think we do recommend visibility for the user.

But this would run on every update, wouldn't it?

But this would run on every update, wouldn't it?

Yes, but we would implement a variable to remember if we have already logged the availability change.

See https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/log-when-unavailable

If internet/device/service is unavailable, log once when unavailable and once when back connected

It looks to me like you never log...

Edit: Or maybe it's because you don't want to log for each entity...

This integration uses partly a DataUpdateCoordinator (https://developers.home-assistant.io/docs/integration_fetching_data?_highlight=fetching&_highlight=data#coordinated-single-api-poll-for-data-for-all-entities) and partly separate polling (https://developers.home-assistant.io/docs/integration_fetching_data?_highlight=fetching&_highlight=data#separate-polling-for-each-individual-entity).
With #131930 implemented we would log the availability for the coordinator. Which should in most cases also apply for the single entities. Maybe we can raise UpdatedFailed in #131930 for some more Exceptions, to get more cases covered?

Thomas55555 · 2024-12-02T17:00:02Z

I've run this fix today. The good thing is, after sunset, only one error message occurs and not one message every 30s, the hole night:

2024-12-02 16:34:04.251 DEBUG (MainThread) [custom_components.apsystems] Finished fetching APSystems Data data in 1.313 seconds (success: True)
2024-12-02 16:34:24.561 ERROR (MainThread) [custom_components.apsystems] Timeout fetching APSystems Data data
2024-12-02 16:34:24.561 DEBUG (MainThread) [custom_components.apsystems] Finished fetching APSystems Data data in 8.624 seconds (success: False)
2024-12-02 16:34:26.561 ERROR (MainThread) [homeassistant.helpers.entity] Update for number.solar_max_output fails
Traceback (most recent call last):
  File "/usr/local/lib/python3.13/site-packages/aiohttp/client.py", line 701, in _request
    conn = await self._connector.connect(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        req, traces=traces, timeout=real_timeout
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/connector.py", line 544, in connect
    proto = await self._create_connection(req, traces, timeout)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/connector.py", line 1050, in _create_connection
    _, proto = await self._create_direct_connection(req, traces, timeout)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/connector.py", line 1363, in _create_direct_connection
    transp, proto = await self._wrap_create_connection(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ...<7 lines>...
    )
    ^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/connector.py", line 1109, in _wrap_create_connection
    sock = await aiohappyeyeballs.start_connection(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ...<5 lines>...
    )
    ^
  File "/usr/local/lib/python3.13/site-packages/aiohappyeyeballs/impl.py", line 82, in start_connection
    sock = await _connect_sock(
           ^^^^^^^^^^^^^^^^^^^^
        current_loop, exceptions, addrinfo, local_addr_infos
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/usr/local/lib/python3.13/site-packages/aiohappyeyeballs/impl.py", line 174, in _connect_sock
    await loop.sock_connect(sock, address)
  File "/usr/local/lib/python3.13/asyncio/selector_events.py", line 641, in sock_connect
    return await fut
           ^^^^^^^^^
asyncio.exceptions.CancelledError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 944, in async_update_ha_state
    await self.async_device_update()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1302, in async_device_update
    await self.async_update()
  File "/config/custom_components/apsystems/number.py", line 49, in async_update
    status = await self._api.get_max_power()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/APsystemsEZ1/__init__.py", line 276, in get_max_power
    response = await self._request("getMaxPower")
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/APsystemsEZ1/__init__.py", line 95, in _request
    async with ses.get(url, timeout=self.timeout) as resp:
               ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/client.py", line 1423, in __aenter__
    self._resp: _RetType = await self._coro
                           ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/client.py", line 607, in _request
    with timer:
         ^^^^^
  File "/usr/local/lib/python3.13/site-packages/aiohttp/helpers.py", line 671, in __exit__
    raise asyncio.TimeoutError from exc_val
TimeoutError
2024-12-02 16:34:45.561 DEBUG (MainThread) [custom_components.apsystems] Finished fetching APSystems Data data in 8.623 seconds (success: False)

I will catch the TimeoutError also.

Thomas55555 · 2024-12-08T18:24:36Z

I've tested this for a few days now. And for mee it seems good.
When the device goes unavailable I get an error message from the coordinator and no uncaught errors from the number entity. Also the number entity now turns unavailable, when the device itself turns unavailable.

frenck

Thanks, @Thomas55555 👍

../Frenck

Thomas55555 added 2 commits December 1, 2024 20:56

Catch ClientConnectorError in APSystems

e034ad2

fix

f519c9e

home-assistant bot added bugfix cla-signed integration: apsystems small-pr PRs with less than 30 lines. Quality Scale: No score labels Dec 1, 2024

epenet added this to the 2024.12.0 milestone Dec 2, 2024

epenet reviewed Dec 2, 2024

View reviewed changes

also catch TimeoutError

27664c9

Thomas55555 marked this pull request as ready for review December 2, 2024 19:44

Thomas55555 marked this pull request as draft December 2, 2024 19:45

Thomas55555 marked this pull request as ready for review December 3, 2024 15:45

Merge branch 'dev' into ClientConnectorError

0b665ae

Thomas55555 and others added 3 commits December 16, 2024 21:57

Merge branch 'dev' into ClientConnectorError

ba82284

Merge branch 'dev' into ClientConnectorError

e224819

add tests

3796ebb

Thomas55555 changed the title ~~Catch ClientConnectorError in APSystems~~ Catch ClientConnectorError ans TimeOutError in APSystems Dec 21, 2024

Thomas55555 changed the title ~~Catch ClientConnectorError ans TimeOutError in APSystems~~ Catch ClientConnectorError and TimeOutError in APSystems Dec 21, 2024

Thomas55555 added 2 commits December 22, 2024 09:44

Merge branch 'dev' into ClientConnectorError

11572eb

Merge branch 'dev' into ClientConnectorError

2386c79

frenck modified the milestones: 2024.12.0, 2025.1.0 Dec 23, 2024

frenck approved these changes Dec 23, 2024

View reviewed changes

frenck merged commit 2f101c5 into home-assistant:dev Dec 23, 2024
33 of 34 checks passed

Thomas55555 deleted the ClientConnectorError branch December 23, 2024 21:50

frenck added the cherry-picked label Dec 24, 2024

frenck pushed a commit that referenced this pull request Dec 24, 2024

Catch ClientConnectorError and TimeOutError in APSystems (#132027)

7ce563b

github-actions bot locked and limited conversation to collaborators Dec 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Catch ClientConnectorError and TimeOutError in APSystems #132027

Catch ClientConnectorError and TimeOutError in APSystems #132027

Thomas55555 commented Dec 1, 2024 •

edited

Loading

home-assistant bot commented Dec 1, 2024

epenet Dec 2, 2024 •

edited

Loading

mawoka-myblock Dec 2, 2024

epenet Dec 2, 2024

mawoka-myblock Dec 2, 2024

Thomas55555 Dec 2, 2024

Thomas55555 Dec 2, 2024 •

edited

Loading

Thomas55555 commented Dec 2, 2024

Thomas55555 commented Dec 8, 2024

frenck left a comment

	try:
	status = await self._api.get_device_power_status()
	except (TimeoutError, ClientConnectionError, InverterReturnedError):
	self._attr_available = False
	else:
	self._attr_available = True
	self._attr_is_on = status

-            self._attr_available = False
+            if self._attr_available:
+                _LOGGER.info(...offline)
+            self._attr_available = False
+        else:
+            if not self._attr_available:
+                _LOGGER.info(...back online)
+            self._attr_available = True

Catch ClientConnectorError and TimeOutError in APSystems #132027

Catch ClientConnectorError and TimeOutError in APSystems #132027

Conversation

Thomas55555 commented Dec 1, 2024 • edited Loading

Breaking change

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Dec 1, 2024

epenet Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

mawoka-myblock Dec 2, 2024

Choose a reason for hiding this comment

epenet Dec 2, 2024

Choose a reason for hiding this comment

mawoka-myblock Dec 2, 2024

Choose a reason for hiding this comment

Thomas55555 Dec 2, 2024

Choose a reason for hiding this comment

Thomas55555 Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Thomas55555 commented Dec 2, 2024

Thomas55555 commented Dec 8, 2024

frenck left a comment

Choose a reason for hiding this comment

Thomas55555 commented Dec 1, 2024 •

edited

Loading

epenet Dec 2, 2024 •

edited

Loading

Thomas55555 Dec 2, 2024 •

edited

Loading