Refactor: Avoid duplicate calls to e.response.json()

[glensc]

Code cleanup after contributor changes: ee7dd4f, d860d01

[coderabbitai]

Walkthrough

Consolidates JSON error parsing in trakt/api.py’s token refresh error handler by parsing once into a variable and reusing it; import formatting switched to multiline. No behavioral changes to public APIs.

Changes

Cohort / File(s)	Summary
Error handling refactor in token refresh trakt/api.py	Replaced repeated e.response.json() calls with a single parsed data = e.response.json() and subsequent data.get(...) lookups; retained existing fallback behavior and logging. Reformatted exception imports into a parenthesized multiline import.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant A as TraktAPI
  participant O as OAuth Server

  C->>A: refresh_token()
  A->>O: POST /token (refresh_token)
  alt Success
    O-->>A: 200 OK (new tokens)
    A-->>C: return tokens
  else Error
    O-->>A: Error response
    A->>A: Parse JSON once into data
    A->>A: Extract error / description from data (fallback if JSON invalid)
    A-->>C: Propagate/handle error (logged)
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled through the error vine,
One parse to rule the JSON line.
No frantic hops from call to call—
A single carrot feeds us all. 🥕
With tidy imports, ears held high,
I thump “refresh!” and swiftly fly.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch token-extract

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (5)

trakt/api.py (5)

260-263: Good: single JSON parse. Also avoid shadowing and broaden decode except.

Parsing once is correct. Minor: this reuses the name data, which already holds the request payload (Lines 246–252). Rename to avoid shadowing, and catch ValueError as well (some JSON decoders raise it).

Apply:

-        data = {
+        payload = {
             'client_id': self.config.CLIENT_ID,
             'client_secret': self.config.CLIENT_SECRET,
             'refresh_token': self.config.OAUTH_REFRESH,
             'redirect_uri': self.REDIRECT_URI,
             'grant_type': 'refresh_token'
         }
@@
-            response = self.client.post('oauth/token', data)
+            response = self.client.post('oauth/token', payload)
@@
-                    data = e.response.json()
-                    error = data.get("error")
-                    error_description = data.get("error_description")
-                except JSONDecodeError:
+                    err_data = e.response.json()
+                    error = err_data.get("error")
+                    error_description = err_data.get("error_description")
+                except (JSONDecodeError, ValueError):
                     error = "Invalid JSON response"
                     error_description = e.response.text

Also applies to: 246-252, 255-255

---

129-133: Avoid sending 'null' bodies; prefer requests' json= param

Current code will send the literal "null" when data is None. Use json= only when payload exists; otherwise omit the body.

-        if method == 'get':  # GETs need to pass data as params, not body
-            response = self.session.request(method, url, headers=self.headers, auth=self.auth, timeout=self.timeout, params=data)
-        else:
-            response = self.session.request(method, url, headers=self.headers, auth=self.auth, timeout=self.timeout, data=json.dumps(data))
+        if method == 'get':  # GETs need to pass data as params, not body
+            response = self.session.request(method, url, headers=self.headers, auth=self.auth, timeout=self.timeout, params=data)
+        else:
+            kwargs = dict(headers=self.headers, auth=self.auth, timeout=self.timeout)
+            if data is not None:
+                kwargs["json"] = data
+            response = self.session.request(method, url, **kwargs)

---

141-145: Propagate the original response in BadResponseException and use response.json()

Using response.json() lets requests handle encoding; include the response in the exception for better diagnostics.

-        try:
-            return json.loads(response.content.decode('UTF-8', 'ignore'))
-        except JSONDecodeError as e:
-            raise BadResponseException(f"Unable to parse JSON: {e}")
+        try:
+            return response.json()
+        except (JSONDecodeError, ValueError) as e:
+            raise BadResponseException(response=response, details=f"Unable to parse JSON: {e}")

---

151-160: Use cached_property instead of @Property + lru_cache

cached_property is simpler and avoids holding references to every instance as lru_cache keys.

-from functools import lru_cache
+from functools import cached_property
@@
-    @property
-    @lru_cache(maxsize=None)
-    def error_map(self):
+    @cached_property
+    def error_map(self):

---

275-279: Defensive check when computing OAUTH_EXPIRES_AT

If created_at or expires_in are missing/None, the addition will raise TypeError. Guard and log.

-        self.config.update(
-            OAUTH_TOKEN=response.get("access_token"),
-            OAUTH_REFRESH=response.get("refresh_token"),
-            OAUTH_EXPIRES_AT=response.get("created_at") + response.get("expires_in"),
-        )
+        created_at = response.get("created_at")
+        expires_in = response.get("expires_in")
+        if not isinstance(created_at, int) or not isinstance(expires_in, int):
+            self.logger.error("Invalid token response: created_at=%r expires_in=%r", created_at, expires_in)
+            return
+        self.config.update(
+            OAUTH_TOKEN=response.get("access_token"),
+            OAUTH_REFRESH=response.get("refresh_token"),
+            OAUTH_EXPIRES_AT=created_at + expires_in,
+        )

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e78678 and 777f45d.

📒 Files selected for processing (1)

• trakt/api.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

trakt/api.py (1)

trakt/errors.py (5)

• BadRequestException (57-61)
• BadResponseException (46-54)
• OAuthException (64-68)
• error (77-78)
• error_description (81-82)

🔇 Additional comments (1)

trakt/api.py (1)

13-14: Import formatting nit — OK to keep

Parenthesized import improves readability; no semantic change.