Skip to content

Harden OAuthRefreshException with robust parsing, clearer formatting, and unit tests#104

Draft
Copilot wants to merge 4 commits into
mainfrom
copilot/split-commit-for-authorization-error
Draft

Harden OAuthRefreshException with robust parsing, clearer formatting, and unit tests#104
Copilot wants to merge 4 commits into
mainfrom
copilot/split-commit-for-authorization-error

Conversation

Copy link
Copy Markdown

Copilot AI commented May 28, 2026

OAuthRefreshException.__init__ unconditionally called self.response.json(), crashing on None response, missing .json(), or non-JSON payloads — common in real error paths.

trakt/errors.py

  • Add message = 'Unauthorized - OAuth token refresh failed'
  • Expand constructor: (response, error, error_description, cause) — explicit args take precedence over parsed response data
  • _load_data() safely parses response JSON, returning {} on missing response, AttributeError, ValueError, or JSONDecodeError
  • __str__ renders message: error - description, message: error, or message depending on available fields
exc = OAuthRefreshException(
    response=response,
    error='invalid_client',
    error_description='client authentication failed',
)
str(exc)
# 'Unauthorized - OAuth token refresh failed: invalid_client - client authentication failed'

str(OAuthRefreshException())
# 'Unauthorized - OAuth token refresh failed'

tests/test_errors.py

  • Tests for default str, JSON-parsed fields, explicit-arg override, None response, .json() raising, error-only str, and cause preservation
Original prompt

Create a new pull request in glensc/python-pytrakt that contains only the changes from commit a5e1448c3cfde70060766ed8aa998135538b7e07 from PR #102, and split the work into the smallest logical commits possible.

Requirements:

  • Base branch: main.
  • Do not include any other commits from PR raise OAuthRefreshException when refresh token fails #102.
  • The target change from commit a5e1448c3cfde70060766ed8aa998135538b7e07 modifies trakt/errors.py only.
  • Recreate that change cleanly on a new branch, but improve it as needed to add unit tests for the added functionality.
  • Keep unrelated changes separate. For example, if there are any docstring/comment-only changes versus behavioral changes versus tests, put them in separate commits whenever possible.
  • Since the selected commit itself is code-only and does not contain docs, split the resulting PR into the smallest sensible commits such as:
    1. Refactor/robustness changes to OAuthRefreshException in trakt/errors.py.
    2. Unit tests covering the new behavior in tests/test_errors.py.
      If an even smaller clean split is possible without making intermediate commits broken or nonsensical, prefer that.

Implementation details derived from the selected commit:

  • Current base version of OAuthRefreshException in trakt/errors.py:
    • __init__(self, response=None) calls super().__init__(response) and then self.data = self.response.json().
    • error property returns self.data["error"].
    • error_description property returns self.data["error_description"].
  • Selected commit changes this exception to be more robust and descriptive:
    • Add a class docstring like Raised when an OAuth access token could not be refreshed.
    • Add message = 'Unauthorized - OAuth token refresh failed'.
    • Update initializer signature to accept response=None, error=None, error_description=None, cause=None.
    • Preserve cause on the instance.
    • Load response JSON safely via a helper (e.g. _load_data) that returns {} when there is no response or when .json() is unavailable/invalid.
    • Populate internal _error and _error_description from explicit args first, then fallback to parsed response data.
    • Make the error and error_description properties return those internal values.
    • Implement __str__ so it returns:
      • <message>: <error> - <error_description> when both are present,
      • <message>: <error> when only error is present,
      • otherwise <message>.
  • Ensure the implementation is idiomatic for this repository and consistent with existing exception classes.

Tests to add:

  • Extend tests/test_errors.py to cover OAuthRefreshException.
  • Add unit tests for at least:
    • default string form when no response/error details are present,
    • parsing error and error_description from a response JSON payload,
    • explicit error / error_description constructor args overriding parsed data,
    • graceful behavior when response is missing or its .json() fails,
    • string formatting when only error exists and when both fields exist.
  • Keep tests focused and minimal; use simple stubs/mocks instead of complex fixtures.

PR expectations:

  • Open a new PR against main.
  • The PR title should clearly indicate this is extracting the OAuthRefreshException improvement into its own PR.
  • In the PR body, mention that this PR was split out from PR raise OAuthRefreshException when refresh token fails #102 and contains only commit a5e1448c3cfde70060766ed8aa998135538b7e07, plus unit tests added for the extracted behavior.
  • Ensure the resulting branch history reflects the smallest logical commits.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Take only this commit @glensc/python-pytrakt/pull/102 (a5e1448) from this pr and create separate PR. the pr should split the commits to smaller chunks as possible. unrelarted. changes (docs for example) and changes that can be committed separately, must be committed separately. addition to the changes, also make unit tests testing the functionality added

Assistant: I’m waiting for your confirmation on the PR creation step so I can continue.

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Refactor OAuthRefreshException and add unit tests Extract OAuthRefreshException hardening from PR #102 (commit a5e1448) with focused unit coverage May 28, 2026
Copilot AI requested a review from glensc May 28, 2026 14:37
Copilot finished work on behalf of glensc May 28, 2026 14:37
Copilot AI added 3 commits May 28, 2026 15:39
- Add class docstring and message attribute
- Expand constructor to accept error, error_description, and cause
- Add _load_data() helper that safely parses response JSON (handles
  missing response, missing .json(), invalid JSON, non-dict payloads)
- Populate _error and _error_description from explicit args first,
  falling back to parsed response data
- Implement __str__ with full, error-only, and fallback formatting
Cover:
- default str form when no response/error details are present
- parsing error and error_description from response JSON payload
- explicit constructor args overriding parsed response data
- graceful behavior when response is None or .json() raises
- str formatting for error-only and error+description cases
- cause attribute preservation
Copilot AI changed the title Extract OAuthRefreshException hardening from PR #102 (commit a5e1448) with focused unit coverage Harden OAuthRefreshException with robust parsing, clearer formatting, and unit tests May 28, 2026
Copilot finished work on behalf of glensc May 28, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants