-
Notifications
You must be signed in to change notification settings - Fork 34
Open
Description
The transact method should raise appropriate exceptions when failures occur. The current implementation catches everything, logs a warning and returns a generic error string.
python-snapcast/snapcast/control/server.py
Lines 157 to 165 in 9c8f97c
| async def _transact(self, method, params=None): | |
| """Wrap requests.""" | |
| result = error = None | |
| try: | |
| result, error = await self._protocol.request(method, params) | |
| except: | |
| _LOGGER.warning('could not send request') | |
| error = 'could not send request' | |
| return result or error |
This makes detecting errors awkward as the type of the return is used to determine if a transaction was successful.
e.g.
python-snapcast/snapcast/control/server.py
Lines 306 to 308 in 9c8f97c
| result = await self._transact(method, params) | |
| if isinstance(result, dict) and key in result: | |
| return result.get(key) |
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 resultJust an idea.
Metadata
Metadata
Assignees
Labels
No labels