Handle ConnectionError

[realumhelp]

Any thoughts on catching a ConnectionError at least when you are doing device discovery. It looks like TransportError is being handled but if a ConnectionError is thrown things kind bomb out. If you are running the test print example (poetry run python -m tedpy) and you put in a host that doesn't exist for example it would seem cleaner to me to raise a ValueError("Host is not a support TED device.") rather than let a ConnectionError stack trace flow out. Probably worth adding some _LOGGER statements in the exception handler as well.

Maybe it is meant to be that way for HA integration, not sure?

The other case that might be useful to catch is if you have a working system that is pulling data and your TED is rebooted or powered off. For some period of time you will receive ConnectionError's until the system is brought back online. This case seems more valid to not catch the exception but I don't know what Python's error handling standards are.

Any thoughts?

[realumhelp]

Specifically referring to the exception being thrown here:

async def _async_fetch_with_retry(self, url: str, **kwargs: Any) -> Any:
        """Retry 3 times to fetch the url if there is a transport error."""
        for attempt in range(3):
            try:
                async with self.async_client as client:
                    return await client.get(url, timeout=30, **kwargs)
            except httpx.TransportError:
                if attempt == 2:
                    raise

[rianadon]

I don't think it matters what particular errors gets thrown, as longs as it's an error. And then Home Assistant can decide to retry later if it would like.

I cleaned up poetry run python -m tedpy in 33b1741accdc87077e63d3dc08c69043c030144b so that it doesn't print so much out on errors.

[realumhelp]

One step ahead of me! It looks better from a print-out perspective. My only concern was during the config-flow that if someone put in "baddomain.local" or something for their TED that it doesn't blow up. I have seen a few Integration do that in the past. The HA part is pretty much a black box to me so I will leave that up to you.

Very excited to see this in HA! If you need anything else let me know I'll see what I can do.

[rianadon]

My only concern was during the config-flow that if someone put in "baddomain.local" or something for their TED that it doesn't blow up.

That's better handled by the integration! The current implementation will tell you it couldn't connect if that happens.

            try:
                reader = await tedpy.createTED(
                    user_input[CONF_HOST], async_client=get_async_client(self.hass)
                )
                await reader.update()
                ...
            except httpx.HTTPError:
                errors["base"] = "cannot_connect"
            except Exception:  # pylint: disable=broad-except
                _LOGGER.exception("Unexpected exception")
                errors["base"] = "unknown"

The only really cool thing I can think of is if there were day & month totals for the mtu consumption. Though I don't remember seeing those when going through the API.

[realumhelp]

I think it is just my C++/Java background that makes worry. It's not immediately obvious what exceptions will come out of createTed() or the check() methods. What if you switched from httpx to a different implementation or something. This is probably more of a Python gripe than anything with the current code base.

As far as the MTU TDY and MTD lemme dig. I might have an idea.

[rianadon]

Yeah Python doesn't have any way of specifying exceptions in code, but it's worth documenting this in the readme. I'm keeping httpx since that's what most Home Assistant integrations use for http communication.