-
Notifications
You must be signed in to change notification settings - Fork 6
Log error when refresh token fails #81
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
Conversation
WalkthroughThe exception handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TokenAuth
participant TraktAPI
Client->>TokenAuth: refresh_token()
TokenAuth->>TraktAPI: Request new token with refresh token
alt Success
TraktAPI-->>TokenAuth: Return new token
TokenAuth-->>Client: Return new token
else OAuthException or BadRequestException
TraktAPI-->>TokenAuth: Raise exception with error details
TokenAuth->>TokenAuth: Log error with status, error, description
TokenAuth-->>Client: Return None (early exit)
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Pull Request Overview
The PR enhances the refresh_token method to log detailed error information when token refreshes fail with HTTP 400 or 401.
- Import
BadRequestExceptionalongside existing exceptions. - Expand the exception handler to catch both
OAuthExceptionandBadRequestException. - Log the HTTP status, error code, and description at the error level.
Comments suppressed due to low confidence (1)
trakt/api.py:252
- This new error-logging branch isn't covered by existing tests. Add unit tests simulating 400 and 401 refresh failures to assert that logger.error is invoked with the proper HTTP code and error details.
except (OAuthException, BadRequestException) as e:
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@glensc automatic tests are not launched due to deprecated |
Refresh token can fail with error 400 or error 401 for many reasons, not only invalid refresh_token.
If refresh fails, an error should be logged.
And response body provides useful information.
Example in PlexTraktSync
Before PR:
After PR:
See :