Closed ccMatrix closed 2 months ago
My take on this: Does this really need more error handling? I mean, this error is so unexpected that I think it's fine if the library crashes in this regard. Any other opinions are highly appreciated.
At the moment it doesn't have any error handling whatsoever. You are just trying to map the return data if it is valid or not. Since there is a message field available in the response why not use it? If message is FAILED
then raise an exception from the library with e.g. RequestFailedError("Could not retrieve data from API"). I can see 6 occurrences of this error today between 13:40 and 19:17 and 3 similar errors for get_device_power_status
. So it does happen frequently enough to handle it properly.
How about just doing a check for message:
return ReturnOutputData(**response["data"]) if response and response.get("message") == "SUCCESS" else None
This is related to the following line of code, where the data is mapped to a ReturnOutputData object if response exists. But it seems that it can happen, that the response was not successful and the data object is None instead of the expected object.
https://github.com/SonnenladenGmbH/APsystems-EZ1-API/blob/e2c83482944a1342287070fbcd05e0e0107b85df/APsystemsEZ1/__init__.py#L165
It might be useful to check for the response.message if it is
"SUCCESS"
and only then map the data to the object. It could then gracefully handle the error case by either retrying the request or throwing a meaningful exception that can be handled in the Home Assistant integration.This is an error response from the getOutputData endpoint which will trigger the above TypeError: