Handle tado update_sensor exceptions #44428

Noltari · 2020-12-21T15:20:36Z

ConnectionError exceptions may also occur when connection is reset by peer.

Signed-off-by: Álvaro Fernández Rojas noltari@gmail.com

Proposed change

The current Tado integration floods the Home Assistant log with the following errors:

2020-12-21 15:20:51 ERROR (MainThread) [homeassistant] Error doing job: Future exception was never retrieved
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 699, in urlopen
    httplib_response = self._make_request(
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 445, in _make_request
    six.raise_from(e, None)
  File "<string>", line 3, in raise_from
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 440, in _make_request
    httplib_response = conn.getresponse()
  File "/usr/local/lib/python3.8/http/client.py", line 1347, in getresponse
    response.begin()
  File "/usr/local/lib/python3.8/http/client.py", line 307, in begin
    version, status, reason = self._read_status()
  File "/usr/local/lib/python3.8/http/client.py", line 268, in _read_status
    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
  File "/usr/local/lib/python3.8/socket.py", line 669, in readinto
    return self._sock.recv_into(b)
  File "/usr/local/lib/python3.8/ssl.py", line 1241, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/local/lib/python3.8/ssl.py", line 1099, in read
    return self._sslobj.read(len, buffer)
ConnectionResetError: [Errno 104] Connection reset by peer

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/requests/adapters.py", line 439, in send
    resp = conn.urlopen(
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 755, in urlopen
    retries = retries.increment(
  File "/usr/local/lib/python3.8/site-packages/urllib3/util/retry.py", line 531, in increment
    raise six.reraise(type(error), error, _stacktrace)
  File "/usr/local/lib/python3.8/site-packages/urllib3/packages/six.py", line 734, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 699, in urlopen
    httplib_response = self._make_request(
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 445, in _make_request
    six.raise_from(e, None)
  File "<string>", line 3, in raise_from
  File "/usr/local/lib/python3.8/site-packages/urllib3/connectionpool.py", line 440, in _make_request
    httplib_response = conn.getresponse()
  File "/usr/local/lib/python3.8/http/client.py", line 1347, in getresponse
    response.begin()
  File "/usr/local/lib/python3.8/http/client.py", line 307, in begin
    version, status, reason = self._read_status()
  File "/usr/local/lib/python3.8/http/client.py", line 268, in _read_status
    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
  File "/usr/local/lib/python3.8/socket.py", line 669, in readinto
    return self._sock.recv_into(b)
  File "/usr/local/lib/python3.8/ssl.py", line 1241, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/local/lib/python3.8/ssl.py", line 1099, in read
    return self._sslobj.read(len, buffer)
urllib3.exceptions.ProtocolError: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/src/homeassistant/homeassistant/components/tado/__init__.py", line 108, in <lambda>
    lambda now: tadoconnector.update(),
  File "/usr/src/homeassistant/homeassistant/util/__init__.py", line 239, in wrapper
    result = method(*args, **kwargs)
  File "/usr/src/homeassistant/homeassistant/components/tado/__init__.py", line 198, in update
    self.update_sensor("zone", zone["id"])
  File "/usr/src/homeassistant/homeassistant/components/tado/__init__.py", line 207, in update_sensor
    data = self.tado.getZoneState(sensor)
  File "/usr/local/lib/python3.8/site-packages/PyTado/interface.py", line 176, in getZoneState
    return TadoZone(self.getState(zone), zone)
  File "/usr/local/lib/python3.8/site-packages/PyTado/interface.py", line 183, in getState
    data = self._apiCall(cmd)
  File "/usr/local/lib/python3.8/site-packages/PyTado/interface.py", line 60, in _apiCall
    self._refresh_token()
  File "/usr/local/lib/python3.8/site-packages/PyTado/interface.py", line 119, in _refresh_token
    response = self._http_session.request("post", url, params=data, timeout=self.timeout, data=json.dumps({}).encode('utf8'),
  File "/usr/local/lib/python3.8/site-packages/requests/sessions.py", line 542, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/local/lib/python3.8/site-packages/requests/sessions.py", line 655, in send
    r = adapter.send(request, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/requests/adapters.py", line 498, in send
    raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))

This happens when tado.getZoneState and tado.getDevices return a "Connection reset by peer" error, which raises an unhandled exception.
With my changes, if this happens, the following will be logged instead:

2020-12-21 15:52:04 ERROR (SyncWorker_9) [custom_components.tado] Unable to connect to Tado while updating zone 1

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)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

# Example configuration.yaml
tado:
  username: !secret tado_username
  password: !secret tado_password

Additional information

This PR fixes or closes issue: fixes Tado integration returning errors #41233
This PR is related to issue: Tado integration returning errors #41233
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
The code has been formatted using Black (black --fast 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.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

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

ConnectionError exceptions may also occur when connection is reset by peer. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

homeassistant · 2020-12-21T15:20:38Z

Hi @Noltari,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

probot-home-assistant · 2020-12-21T15:21:35Z

Hey there @michaelarnauts, @bdraco, mind taking a look at this pull request as its been labeled with an integration (tado) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

frenck · 2020-12-21T15:25:13Z

homeassistant/components/tado/__init__.py

@@ -215,7 +215,7 @@ def update_sensor(self, sensor_type, sensor):
            else:
                _LOGGER.debug("Unknown sensor: %s", sensor_type)
                return
-        except RuntimeError:
+        except:


In my opinion, catching all exceptions broadly is a bad practice.
I think it is better if the upstream package handles the possible exceptions better and would throw specific errors we can handle correctly.

Have you considered contributing an improvement upstream?

In my opinion, catching all exceptions broadly is a bad practice.

If you want I can update the PR catching only ConnectionError.

I think it is better if the upstream package handles the possible exceptions better and would throw specific errors we can handle correctly.

Sure, but that's up to the upstream maintainers.

Have you considered contributing an improvement upstream?

~~Unfortunately I don't have the time to do that, so I've created an issue upstream.~~
EDIT: it was easier than I thought, so I created a PR upstream: wmalgadey/PyTado#43

Noltari · 2020-12-23T10:06:49Z

Closing, as this should be fixed upstream:
wmalgadey/PyTado#43
wmalgadey/PyTado#42

tado: handle update_sensor exceptions

5a7637f

ConnectionError exceptions may also occur when connection is reset by peer. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

homeassistant added the cla-needed label Dec 21, 2020

probot-home-assistant bot added bugfix integration: tado small-pr PRs with less than 30 lines. labels Dec 21, 2020

probot-home-assistant bot assigned bdraco and michaelarnauts Dec 21, 2020

project-bot bot added this to Needs review in Dev Dec 21, 2020

homeassistant added cla-signed and removed cla-needed labels Dec 21, 2020

frenck reviewed Dec 21, 2020

View reviewed changes

Noltari mentioned this pull request Dec 21, 2020

_refresh_token connection reset exception wmalgadey/PyTado#42

Closed

MartinHjelmare changed the title ~~tado: handle update_sensor exceptions~~ Handle tado update_sensor exceptions Dec 22, 2020

Noltari closed this Dec 23, 2020

Dev automation moved this from Needs review to Cancelled Dec 23, 2020

Noltari mentioned this pull request Dec 23, 2020

Add full list of Tado devices (and battery state) #44475

Merged

21 tasks

github-actions bot locked and limited conversation to collaborators Dec 24, 2020

Noltari deleted the tado-exception branch December 28, 2020 09:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle tado update_sensor exceptions #44428

Handle tado update_sensor exceptions #44428

Noltari commented Dec 21, 2020

homeassistant commented Dec 21, 2020

probot-home-assistant bot commented Dec 21, 2020

frenck Dec 21, 2020

Noltari Dec 21, 2020 •

edited

Loading

Noltari commented Dec 23, 2020

Handle tado update_sensor exceptions #44428

Handle tado update_sensor exceptions #44428

Conversation

Noltari commented Dec 21, 2020

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

homeassistant commented Dec 21, 2020

probot-home-assistant bot commented Dec 21, 2020

frenck Dec 21, 2020

Choose a reason for hiding this comment

Noltari Dec 21, 2020 • edited Loading

Choose a reason for hiding this comment

Noltari commented Dec 23, 2020

Example entry for `configuration.yaml`:

Noltari Dec 21, 2020 •

edited

Loading