python-snapcast
python-snapcast copied to clipboard
Failed transactions should raise appropriate exceptions
The transact method should raise appropriate exceptions when failures occur. The current implementation catches everything, logs a warning and returns a generic error string.
https://github.com/happyleavesaoc/python-snapcast/blob/9c8f97cea23015ab2414e9eee43926eca5878634/snapcast/control/server.py#L157-L165
This makes detecting errors awkward as the type of the return is used to determine if a transaction was successful. e.g. https://github.com/happyleavesaoc/python-snapcast/blob/9c8f97cea23015ab2414e9eee43926eca5878634/snapcast/control/server.py#L306-L308
I think it would be better for this method to raise the exception, possibly a custom one.
try:
result, error = await self._protocol.request(method, params)
except OSError as e:
_LOGGER.warning('Could not send request: %s', e)
raise RequestException(e) from e
return result
Just an idea.
I agree, the implementation could be better. I am working on this HA bug: https://github.com/home-assistant/core/issues/106374 which might be related. So I could look into this as well.