Improve error handling in user authentication#36
Improve error handling in user authentication#36deltamikeromeo wants to merge 2 commits intolawtancool:masterfrom
Conversation
… (Unauthorized, NotFound, BadCredentials, InvalidCategory, C4CorruptXMLResponse). Implements checkResponseForError(response_text) to: Detect and raise exceptions for known error messages in plain text, JSON, or XML responses from the Control4 Director. Handles malformed XML responses and logs parsing errors. Raises specific exceptions based on error codes or messages found in the response.
|
In relation to #35 |
lawtancool
left a comment
lawtancool
left a comment
There was a problem hiding this comment.
Thanks for submitting this PR! Overall your approach of decoding it as JSON, then trying it as XML makes sense. Just requested a few changes to make it better match the old behavior.
| raise NotFound("404 Not Found from Control4 Director.") | ||
| if "error" in dictionary: | ||
| if "Invalid category" in dictionary["error"]: | ||
| raise InvalidCategory(dictionary["error"]) |
There was a problem hiding this comment.
In all the exception cases in this function, can you change the behavior so that it includes the entire response_text in the thrown exception? This would match the old behavior and make sure the entire response gets printed in the error logs
| # Other error codes can be added here if necessary | ||
| else: | ||
| # Generic error for other codes | ||
| raise Exception( |
There was a problem hiding this comment.
Raise C4Exception instead to match old behavior
| _LOGGER.error( | ||
| ( | ||
| "Failed to parse XML response from Director due to a mismatched tag or other corruption. " | ||
| "The raw text received from the controller was: \n%s" | ||
| ), | ||
| response_text, | ||
| ) | ||
| # Re-raise the original error so the integration still fails as expected | ||
| raise e |
There was a problem hiding this comment.
Instead of using _LOGGER is there a way to include this error message plus the ExpatError inside a C4CorruptXMLResponse instead? Right now it seems that C4CorruptXMLResponse is completely unused.
2 participants
Enhance user authentication by adding more robust error handling, ensuring clearer feedback for users during login failures.
Defines custom exception classes for various Control4 error scenarios (Unauthorized, NotFound, BadCredentials, InvalidCategory, C4CorruptXMLResponse).
Implements checkResponseForError(response_text) to:
Detect and raise exceptions for known error messages in plain text, JSON, or XML responses from the Control4 Director.
Handles malformed XML responses and logs parsing errors.
Raises specific exceptions based on error codes or messages found in the response.