Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't fail when HAFAS returns an Exception #14

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

evgeni
Copy link
Contributor

@evgeni evgeni commented Oct 4, 2024

When there are no journeys found, pyHAFAS throws an exception.
Let's catch it, instead of spamming the logs :)

2024-10-04 10:16:40.247 ERROR (MainThread) [homeassistant.components.sensor] hafas: Error on device update!
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 724, in _async_add_entity
    await entity.async_device_update(warning=False)
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1300, in async_device_update
    await self.async_update()
  File "/config/custom_components/hafas/sensor.py", line 107, in async_update
    self.journeys = await self.hass.async_add_executor_job(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/deps/lib/python3.12/site-packages/pyhafas/client.py", line 157, in journeys
    res = self.profile.request(body)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/deps/lib/python3.12/site-packages/pyhafas/profile/base/helper/request.py", line 96, in request
    return HafasResponse(res, BaseErrorCodesMapping)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/deps/lib/python3.12/site-packages/pyhafas/types/hafas_response.py", line 30, in __init__
    self.check_for_errors(mapping)
  File "/config/deps/lib/python3.12/site-packages/pyhafas/types/hafas_response.py", line 53, in check_for_errors
    raise mapping[self.data['svcResL'][0]['err']].value(
pyhafas.types.exceptions.JourneysArrivalDepartureTooNearError: HAFAS Kernel: No connection found

I've chosen to catch all exceptions, as pyhafas Exceptions don't have
any base class (besides Exception) and I thought it's nicer to report no
journeys for any error case.

@akloeckner
Copy link
Owner

Thanks! That's a good idea. Should we still issue a one-line warning? If we simply fail silently, the user might think, there simply are no connections...

@evgeni
Copy link
Contributor Author

evgeni commented Oct 4, 2024

You mean in the log? We certainly could.

The current code makes the UI render the state as "Unknown", which for me is sufficient, but not all users are me ;)

Any preferred wording? Or just a:

except Exception as e:
    _LOGGER.warning(f"Couldn't fetch journeys: {e}")

@evgeni evgeni force-pushed the dont-fail-no-connections branch from bc9db09 to 7114b94 Compare October 4, 2024 19:07
@akloeckner
Copy link
Owner

Yes, I meant in the log. I think unknown is the correct state, if there are no trips.

I suggest to also include the entity ID in the message, since that's where the user could check the configuration. Such that something like this results:

2024-10-04 10:16:40.247 WARNING (MainThread) [homeassistant.components.sensor.hafas] Couldn't fetch journeys for sensor.a_to_b: pyhafas.types.exceptions.JourneysArrivalDepartureTooNearError

Would that make sense?

@evgeni
Copy link
Contributor Author

evgeni commented Oct 5, 2024

that'd be self.entity_id, right? (first time touching HA internals)

@akloeckner
Copy link
Owner

I think so, yes. self.entity_id seems to be provided by the Entity base class.

When there are no journeys found, pyHAFAS throws an exception.
Let's catch it, instead of spamming the logs :)

```
2024-10-04 10:16:40.247 ERROR (MainThread) [homeassistant.components.sensor] hafas: Error on device update!
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 724, in _async_add_entity
    await entity.async_device_update(warning=False)
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1300, in async_device_update
    await self.async_update()
  File "/config/custom_components/hafas/sensor.py", line 107, in async_update
    self.journeys = await self.hass.async_add_executor_job(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/deps/lib/python3.12/site-packages/pyhafas/client.py", line 157, in journeys
    res = self.profile.request(body)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/deps/lib/python3.12/site-packages/pyhafas/profile/base/helper/request.py", line 96, in request
    return HafasResponse(res, BaseErrorCodesMapping)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/deps/lib/python3.12/site-packages/pyhafas/types/hafas_response.py", line 30, in __init__
    self.check_for_errors(mapping)
  File "/config/deps/lib/python3.12/site-packages/pyhafas/types/hafas_response.py", line 53, in check_for_errors
    raise mapping[self.data['svcResL'][0]['err']].value(
pyhafas.types.exceptions.JourneysArrivalDepartureTooNearError: HAFAS Kernel: No connection found
```

I've chosen to catch *all* exceptions, as pyhafas Exceptions don't have
any base class (besides Exception) and I thought it's nicer to report no
journeys for any error case.
@evgeni evgeni force-pushed the dont-fail-no-connections branch from 7114b94 to 3e60036 Compare October 5, 2024 14:54
Copy link
Owner

@akloeckner akloeckner left a comment

Choose a reason for hiding this comment

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

Thanks! This seems to work great here, too. It looks much tidier in the logs.

@akloeckner akloeckner merged commit d2ff875 into akloeckner:master Oct 5, 2024
2 checks passed
@akloeckner
Copy link
Owner

I just realized, yours is the first community contribution here. 🎉

Thanks again. I just created a new release with the change.

@evgeni evgeni deleted the dont-fail-no-connections branch October 5, 2024 20:30
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.

2 participants