Fix KeyError in User.__init__ and error handling (#417)#421
Fix KeyError in User.__init__ and error handling (#417)#421PriyanshAroraa wants to merge 2 commits into
Conversation
X's API response omits certain legacy user fields intermittently (entities.description.urls, withheld_in_countries, etc.), causing User.__init__ to crash with KeyError. Error responses from X sometimes omit the 'code' field too, which masks the actual failure with a confusing KeyError in client.request. Switch all direct dict lookups in User.__init__ and the error-code read in Client.request to .get() with sensible defaults (empty string for strings, 0 for counts, False for booleans, empty list for list fields). No behavior change when fields are present; graceful fallback when they are not.
Reviewer's GuideReplaces brittle direct dict indexing in User model initialization and error handling with safe .get() accessors and sensible defaults to prevent intermittent KeyErrors when the X API omits optional fields. Class diagram for updated User and Client.request behaviorclassDiagram
class Client {
+request(method, url, params, json_body, files, headers) async
}
class User {
+Client client
+dict data
+str id
+str created_at
+str name
+str screen_name
+str profile_image_url
+str profile_banner_url
+str url
+str location
+str description
+list description_urls
+list urls
+list pinned_tweet_ids
+bool is_blue_verified
+bool verified
+bool possibly_sensitive
+bool can_dm
+bool can_media_tag
+bool want_retweets
+bool default_profile
+bool default_profile_image
+bool has_custom_timelines
+int followers_count
+int fast_followers_count
+int normal_followers_count
+int following_count
+int favourites_count
+int listed_count
+int media_count
+int statuses_count
+bool is_translator
+str translator_type
+list withheld_in_countries
+bool protected
+__init__(client, data)
}
Client "1" --> "many" User : creates
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes make JSON parsing defensive: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
Client.request, you now tolerate missingcodekeys but still assumeerrors[0]exists; consider guarding against an emptyerrorslist (or non-list) before indexing to avoidIndexErrorin these cases. - In
User.__init__, silently defaulting all missing numeric/boolean fields to0/Falsemay hide upstream API changes; you might want to selectively allowNonefor fields where absence is meaningfully different from zero/false, or document the intentional fallback semantics in the type hints (e.g.,Optional[int]). - The repeated
.get(..., default)pattern inUser.__init__could be refactored into a small helper or mapping of field names to defaults to reduce duplication and make future changes to default behavior easier to manage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Client.request`, you now tolerate missing `code` keys but still assume `errors[0]` exists; consider guarding against an empty `errors` list (or non-list) before indexing to avoid `IndexError` in these cases.
- In `User.__init__`, silently defaulting all missing numeric/boolean fields to `0`/`False` may hide upstream API changes; you might want to selectively allow `None` for fields where absence is meaningfully different from zero/false, or document the intentional fallback semantics in the type hints (e.g., `Optional[int]`).
- The repeated `.get(..., default)` pattern in `User.__init__` could be refactored into a small helper or mapping of field names to defaults to reduce duplication and make future changes to default behavior easier to manage.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/user.py`:
- Line 94: The constructor currently sets self.created_at: str =
legacy.get('created_at', '') which leaves created_at as an empty string and
causes User.created_at_datetime to pass that into timestamp_to_datetime() and
raise ValueError; change created_at to be nullable (e.g., Optional[str]) and
update created_at assignment to allow None (use legacy.get('created_at') without
default or explicit None), then modify created_at_datetime to guard the fallback
by returning None if self.created_at is falsy/None or only calling
timestamp_to_datetime(self.created_at) when self.created_at is a non-empty
value; reference symbols: created_at attribute on the User class,
created_at_datetime property, and timestamp_to_datetime().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ad15fe8-e97a-4d68-812e-0fc7b415b2a4
📒 Files selected for processing (2)
twikit/client/client.pytwikit/user.py
| self.name: str = legacy['name'] | ||
| self.screen_name: str = legacy['screen_name'] | ||
| self.profile_image_url: str = legacy['profile_image_url_https'] | ||
| self.created_at: str = legacy.get('created_at', '') |
There was a problem hiding this comment.
Handle missing created_at in created_at_datetime too.
Defaulting created_at to '' avoids the constructor KeyError, but User.created_at_datetime still passes that empty string to timestamp_to_datetime(), which will raise ValueError. Please make the property nullable or otherwise guard the fallback.
🐛 Proposed fix
- self.created_at: str = legacy.get('created_at', '')
+ self.created_at: str = legacy.get('created_at', '')
@@
- def created_at_datetime(self) -> datetime:
- return timestamp_to_datetime(self.created_at)
+ def created_at_datetime(self) -> datetime | None:
+ if not self.created_at:
+ return None
+ return timestamp_to_datetime(self.created_at)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.created_at: str = legacy.get('created_at', '') | |
| self.created_at: str = legacy.get('created_at', '') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@twikit/user.py` at line 94, The constructor currently sets self.created_at:
str = legacy.get('created_at', '') which leaves created_at as an empty string
and causes User.created_at_datetime to pass that into timestamp_to_datetime()
and raise ValueError; change created_at to be nullable (e.g., Optional[str]) and
update created_at assignment to allow None (use legacy.get('created_at') without
default or explicit None), then modify created_at_datetime to guard the fallback
by returning None if self.created_at is falsy/None or only calling
timestamp_to_datetime(self.created_at) when self.created_at is a non-empty
value; reference symbols: created_at attribute on the User class,
created_at_datetime property, and timestamp_to_datetime().
Per Sourcery review on d60#421: the previous patch handled missing 'code' on the first error, but still assumed errors[0] exists and is a dict. X can return errors as an empty list or a non-list; guard against both before indexing.
|
@sourcery-ai thanks for the review. Addressed the real one, leaving the others as call-outs for the maintainer: 1. Guarding errors = response_data.get('errors') if isinstance(response_data, dict) else None
if errors and isinstance(errors, list):
first_error = errors[0] if isinstance(errors[0], dict) else {}
error_code = first_error.get('code')
error_message = first_error.get('message')2. 3. Helper for the repeated |
|
Duplicate of #418 which was opened two days before this. Please close this PR. |
|
@PawiX25 thanks for the heads up, missed #418 in my search. Closing as a duplicate - yours is the correct one. One thing my PR picked up from the Sourcery review that might be worth adding to #418: guarding against empty or non-list |
2 participants
Fixes #417.
Problem
Two separate
KeyErrorcrashes when X's API returns responses with missing fields:1.
User.__init__— crashes onlegacy['entities']['description']['urls'],legacy['withheld_in_countries'], and any of the ~30 other fields accessed via direct dict lookup. X's API intermittently omits these for certain accounts.2.
Client.request—response_data['errors'][0]['code']can KeyError because X sometimes returns error objects without acodefield. This masks the real failure with a confusing KeyError during error handling itself.Fix
Switch direct dict lookups to
.get()with sensible defaults:location,description, etc.)0for counts (followers_count,media_count, etc.)Falsefor booleans (verified,is_translator, etc.)pinned_tweet_ids,withheld_in_countries,description_urls,urls)Noneforerror_code(matches the existingerror_messageline which already uses.get())No behavior change when fields are present; graceful fallback when they are not.
Test
Hit the bug in production polling 50+ accounts across multiple niches. With the patch,
get_user_by_screen_nameandget_user_tweetshave run cleanly across thousands of calls — no KeyErrors.Summary by Sourcery
Handle missing fields in X API responses without raising KeyError in user initialization and error handling.
Bug Fixes:
Summary by CodeRabbit