Improve error handling in user authentication #36
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… (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
requested changes
Aug 23, 2025
| 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
lawtancool
reviewed
Aug 23, 2025
Comment on lines
+65
to
+73
| _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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.