-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved error checks #377
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new error handling mechanism for API responses, specifically checking for 'UnauthorizedException' and other API errors. It adds a new method Sequence diagram for improved API error handlingsequenceDiagram
participant Client
participant API
participant ErrorHandler
Client->>API: Make API request
API-->>ErrorHandler: Check response errors
alt Has errors
alt UnauthorizedException
ErrorHandler-->>Client: Raise AuthFailedError
else Other error
ErrorHandler-->>Client: Raise ApiError
end
else No errors
ErrorHandler-->>Client: Return True
end
Class diagram showing new error handling methodclassDiagram
class NiceGoApi {
-_check_response_errors(response: dict) bool
+get_all_barriers() list[Barrier]
+open_barrier(barrier_id: str) bool
+close_barrier(barrier_id: str) bool
+light_on(barrier_id: str) bool
+light_off(barrier_id: str) bool
}
class AuthFailedError {
}
class ApiError {
}
NiceGoApi ..> AuthFailedError : throws
NiceGoApi ..> ApiError : throws
note for NiceGoApi "Added _check_response_errors method
to handle API error responses"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @IceBotYT - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add tests to verify the error handling behavior, particularly for authentication failures and other API errors
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #377 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 15 15
Lines 1173 1194 +21
=========================================
+ Hits 1173 1194 +21 ☔ View full report in Codecov by Sentry. |
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @IceBotYT - I've reviewed your changes - here's some feedback:
Overall Comments:
- The PR includes tests but the checklist item for tests is not checked - please update the checklist
- Since this fixes a documented issue (#126370), please add an entry to the CHANGELOG.md
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Description
Improved error checks. Should help with home-assistant/core#126370
Checklist
CHANGELOG.md
OR changes are insignificantSummary by Sourcery
Implement error handling for GraphQL API responses. Handle "UnauthorizedException" errors and raise an "AuthFailedError".
Bug Fixes:
Tests: