Fix fetchById issue of target location#405
Conversation
This is the problem regarding the fetching of target with a X id and then it returns ItemContent... this is fixed and working as intended
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@twikit/client/client.py`:
- Line 1526: The pagination cursor extraction currently assumes
entries[-1]['content']['value'] and later
entries[-1]['content']['itemContent']['value'] which can KeyError if payload
shape varies; update the extraction used for next_cursor (symbol: next_cursor
and the entries list) to be schema‑tolerant: attempt content['value'], then
content.get('itemContent', {}).get('value') (or equivalent safe lookups),
falling back to None if neither exists, and apply the same tolerant fallback
wherever content.itemContent.value is read (the later read around line with
content.itemContent.value) so both paths handle either shape without raising
KeyError.
|
|
||
| if entries[-1]['entryId'].startswith('cursor'): | ||
| next_cursor = entries[-1]['content']['itemContent']['value'] | ||
| next_cursor = entries[-1]['content']['value'] |
There was a problem hiding this comment.
Use schema-tolerant cursor extraction here to avoid pagination breakage.
Line 1526 hardcodes content.value, but the same tweet-detail flow still reads content.itemContent.value at Line 1635. If payload shape differs between pages, one path can fail with KeyError and break reply pagination.
🔧 Proposed hardening
- if entries[-1]['entryId'].startswith('cursor'):
- next_cursor = entries[-1]['content']['value']
+ if entries[-1]['entryId'].startswith('cursor'):
+ cursor_content = entries[-1].get('content', {})
+ next_cursor = (
+ cursor_content.get('value')
+ or cursor_content.get('itemContent', {}).get('value')
+ )
+ if next_cursor is None:
+ _fetch_next_result = None
+ return Result(results, _fetch_next_result, None)
_fetch_next_result = partial(self._get_more_replies, tweet_id, next_cursor)Also apply the same fallback pattern at Line 1635 for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@twikit/client/client.py` at line 1526, The pagination cursor extraction
currently assumes entries[-1]['content']['value'] and later
entries[-1]['content']['itemContent']['value'] which can KeyError if payload
shape varies; update the extraction used for next_cursor (symbol: next_cursor
and the entries list) to be schema‑tolerant: attempt content['value'], then
content.get('itemContent', {}).get('value') (or equivalent safe lookups),
falling back to None if neither exists, and apply the same tolerant fallback
wherever content.itemContent.value is read (the later read around line with
content.itemContent.value) so both paths handle either shape without raising
KeyError.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts how the next pagination cursor is read when loading more replies, fixing incorrect parsing of the API response that caused wrong content to be fetched. Sequence diagram for loading additional replies with corrected cursor parsingsequenceDiagram
actor User
participant Client
participant TwitterAPI
User->>Client: load_more_replies(tweet_id)
Client->>TwitterAPI: GET replies(tweet_id, cursor)
TwitterAPI-->>Client: response(entries)
Client->>Client: inspect last_entry = entries[-1]
alt last_entry.entryId starts with cursor
Client->>Client: next_cursor = last_entry.content.value
Client->>TwitterAPI: GET replies(tweet_id, next_cursor)
TwitterAPI-->>Client: additional replies
else no cursor entry
Client->>Client: next_cursor = None
end
Client-->>User: replies list
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The change assumes
entries[-1]['content']['value']always exists; consider using a safer access pattern (e.g.,getwith a clear error or fallback) to avoidKeyErrorif the API response shape changes again. - If the API can sometimes return
itemContentand sometimes not, you might want to handle both shapes (checking foritemContentfirst, thenvalue) for backward compatibility rather than hard-switching to the new structure.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change assumes `entries[-1]['content']['value']` always exists; consider using a safer access pattern (e.g., `get` with a clear error or fallback) to avoid `KeyError` if the API response shape changes again.
- If the API can sometimes return `itemContent` and sometimes not, you might want to handle both shapes (checking for `itemContent` first, then `value`) for backward compatibility rather than hard-switching to the new structure.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This is the problem regarding the fetching of target with a X id and then it returns ItemContent... this is fixed and working as intended
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit