Fix KEY_BYTE indices parsing + guard optional legacy user fields - #432
Open
leslienwu wants to merge 4 commits into
Open
Fix KEY_BYTE indices parsing + guard optional legacy user fields#432leslienwu wants to merge 4 commits into
leslienwu wants to merge 4 commits into
Conversation
X changed how it references the ondemand.s.*.js bundle in the homepage HTML, breaking ON_DEMAND_FILE_REGEX and causing "Couldn't get KEY_BYTE indices" on every request (see d60#408, d60#409). Adapted from d60#411 by @ryanstoic, which switches to a two-step lookup: find the webpack chunk index for "ondemand.s", then look up that chunk's hash separately. Also hardened ON_DEMAND_HASH_PATTERN to accept both single- and double-quoted hash values (the original PR only handled double quotes, which left the same failure mode for single-quoted chunk data).
X omits entities.description.urls and withheld_in_countries from the
legacy user payload for some accounts (observed on accounts with no
links in their bio). Direct dict indexing on these fields raised
KeyError('urls') / KeyError('withheld_in_countries') and aborted the
entire tweet/user fetch.
Switched both fields in User (twikit/user.py) and the guest-mode
User (twikit/guest/user.py) to .get(...) with safe defaults, matching
the pattern already used for sibling optional fields like
profile_banner_url and protected in the same classes.
Adds an unmissable banner at the top of the README so anyone landing on this fork understands it's an unofficial compatibility patch, what it fixes relative to upstream, and how to pip install it directly.
Reviewer's GuideUpdates the client transaction logic to match X’s new ondemand webpack chunk format and hardens user parsing against missing legacy fields, plus documents this fork as a temporary patch in the README. Sequence diagram for updated get_indices ondemand chunk lookupsequenceDiagram
participant ClientTransaction
participant Homepage as HomepageResponse
participant Session
participant AbsTwimg as AbsTwimgServer
ClientTransaction->>ClientTransaction: get_indices(home_page_response, session, headers)
ClientTransaction->>ClientTransaction: validate_response(home_page_response)
ClientTransaction->>ClientTransaction: ON_DEMAND_FILE_REGEX.search(response)
alt ondemand_chunk_found
ClientTransaction->>ClientTransaction: re.search(ON_DEMAND_HASH_PATTERN.format(chunk_index), response)
alt hash_found
ClientTransaction->>AbsTwimg: session.request(GET, on_demand_file_url, headers)
AbsTwimg-->>ClientTransaction: on_demand_file_response
ClientTransaction->>ClientTransaction: INDICES_REGEX.finditer(on_demand_file_response.text)
ClientTransaction->>ClientTransaction: key_byte_indices.append(group(1))
else hash_not_found
ClientTransaction->>ClientTransaction: [key_byte_indices remains empty]
end
else ondemand_chunk_not_found
ClientTransaction->>ClientTransaction: [key_byte_indices remains empty]
end
ClientTransaction->>ClientTransaction: [if not key_byte_indices: raise Exception]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe patch updates X frontend transaction parsing, makes selected user fields tolerant of missing response data, and documents the unofficial fork and its installation command. ChangesFrontend compatibility and user data handling
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ClientTransaction
participant HomePage
participant OndemandScript
ClientTransaction->>HomePage: Fetch home page source
ClientTransaction->>HomePage: Extract chunk id and file hash
ClientTransaction->>OndemandScript: Fetch hashed ondemand script
ClientTransaction->>OndemandScript: Extract animation indices
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 found 1 issue, and left some high level feedback:
- In
get_indices, consider operating directly onhome_page_response.textinstead ofstr(response)to avoid relying on the string representation of the response object and make the intent of the regex parsing clearer. - The new
ON_DEMAND_FILE_REGEXandON_DEMAND_HASH_PATTERNare tightly coupled to a specific bundle layout; adding a short comment describing the expected HTML/JSON snippet format would make future maintenance and adjustments to X’s changes much easier. - The logic for optional user fields (
description_urls,withheld_in_countries) is duplicated betweenuser.pyandguest/user.py; consider extracting a small helper or shared initializer to keep this behavior consistent and reduce the risk of them drifting apart.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_indices`, consider operating directly on `home_page_response.text` instead of `str(response)` to avoid relying on the string representation of the response object and make the intent of the regex parsing clearer.
- The new `ON_DEMAND_FILE_REGEX` and `ON_DEMAND_HASH_PATTERN` are tightly coupled to a specific bundle layout; adding a short comment describing the expected HTML/JSON snippet format would make future maintenance and adjustments to X’s changes much easier.
- The logic for optional user fields (`description_urls`, `withheld_in_countries`) is duplicated between `user.py` and `guest/user.py`; consider extracting a small helper or shared initializer to keep this behavior consistent and reduce the risk of them drifting apart.
## Individual Comments
### Comment 1
<location path="twikit/guest/user.py" line_range="96-97" />
<code_context>
self.location: str = legacy['location']
self.description: str = legacy['description']
- self.description_urls: list = legacy['entities']['description']['urls']
+ self.description_urls: list = legacy['entities'].get('description', {}).get('urls', [])
self.urls: list = legacy['entities'].get('url', {}).get('urls')
self.pinned_tweet_ids: list[str] = legacy['pinned_tweet_ids_str']
self.is_blue_verified: bool = data['is_blue_verified']
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a consistent default for `urls` to avoid `None` vs `[]` inconsistencies.
`description_urls` now defaults to `[]`, but `urls` still becomes `None` when `legacy['entities']['url']` is missing. This mismatch can break code that assumes both are lists. Please also default `urls` to an empty list, e.g.:
```python
self.urls: list = legacy['entities'].get('url', {}).get('urls', [])
```
The same adjustment likely applies in the `User` class to keep behavior consistent there as well.
Suggested implementation:
```python
self.description_urls: list = legacy['entities'].get('description', {}).get('urls', [])
self.urls: list = legacy['entities'].get('url', {}).get('urls', [])
```
To fully apply your suggestion, wherever the main `User` class (non-guest) assigns `self.urls` from `legacy['entities']['url']['urls']`, adjust it to:
```python
self.urls: list = legacy['entities'].get('url', {}).get('urls', [])
```
This ensures both guest users and regular users consistently default `urls` to `[]` instead of `None`, avoiding list vs `None` inconsistencies in downstream code.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+96
to
97
| self.description_urls: list = legacy['entities'].get('description', {}).get('urls', []) | ||
| self.urls: list = legacy['entities'].get('url', {}).get('urls') |
There was a problem hiding this comment.
suggestion (bug_risk): Use a consistent default for urls to avoid None vs [] inconsistencies.
description_urls now defaults to [], but urls still becomes None when legacy['entities']['url'] is missing. This mismatch can break code that assumes both are lists. Please also default urls to an empty list, e.g.:
self.urls: list = legacy['entities'].get('url', {}).get('urls', [])The same adjustment likely applies in the User class to keep behavior consistent there as well.
Suggested implementation:
self.description_urls: list = legacy['entities'].get('description', {}).get('urls', [])
self.urls: list = legacy['entities'].get('url', {}).get('urls', [])To fully apply your suggestion, wherever the main User class (non-guest) assigns self.urls from legacy['entities']['url']['urls'], adjust it to:
self.urls: list = legacy['entities'].get('url', {}).get('urls', [])This ensures both guest users and regular users consistently default urls to [] instead of None, avoiding list vs None inconsistencies in downstream code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 1-28: Update the README banner’s install command fence to specify
the bash language, and remove the trailing empty blockquote marker after the
closing fence so the blockquote has no blank line. Preserve the existing command
and surrounding text unchanged.
In `@twikit/guest/user.py`:
- Line 96: Update the description_urls initialization in the User construction
flow to handle legacy['entities'] being None before calling get. Preserve the
existing empty-list fallback when the entities or description data is absent,
while retaining URL extraction for populated entities.
- Line 115: Update the withheld_in_countries initialization in the user data
construction flow to normalize both missing and None values to an empty list,
while preserving any provided country list. Use the existing legacy value
retrieval rather than introducing additional state.
In `@twikit/user.py`:
- Line 124: Update the `withheld_in_countries` assignment in the user model to
use the legacy value when present but fall back to an empty list when it is
`None`, matching the handling in `twikit/guest/user.py`.
- Line 102: Update the user initialization logic around description_urls to
handle legacy['entities'] being None before calling .get(), matching the
defensive handling already implemented in twikit/guest/user.py. Preserve the
existing empty URL-list fallback when entities or description data is absent.
🪄 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: 4ab6c920-d511-40f0-9921-7453b9ec95d6
📒 Files selected for processing (4)
README.mdtwikit/guest/user.pytwikit/user.pytwikit/x_client_transaction/transaction.py
| self.location: str = legacy['location'] | ||
| self.description: str = legacy['description'] | ||
| self.description_urls: list = legacy['entities']['description']['urls'] | ||
| self.description_urls: list = legacy['entities'].get('description', {}).get('urls', []) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
legacy['entities'] can be None, causing AttributeError.
The upstream build_user_data (in twikit/utils.py) constructs legacy with 'entities': raw_data.get('entities'), so the entities key is always present but its value can be None. When that happens, legacy['entities'].get(...) raises AttributeError: 'NoneType' object has no attribute 'get' — the very crash this PR intends to prevent.
🛡️ Proposed fix: use safe access for the entities key
- self.description_urls: list = legacy['entities'].get('description', {}).get('urls', [])
+ self.description_urls: list = (legacy.get('entities') or {}).get('description', {}).get('urls', [])📝 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.
Suggested change
| self.description_urls: list = legacy['entities'].get('description', {}).get('urls', []) | |
| self.description_urls: list = (legacy.get('entities') or {}).get('description', {}).get('urls', []) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@twikit/guest/user.py` at line 96, Update the description_urls initialization
in the User construction flow to handle legacy['entities'] being None before
calling get. Preserve the existing empty-list fallback when the entities or
description data is absent, while retaining URL extraction for populated
entities.
| self.is_translator: bool = legacy['is_translator'] | ||
| self.translator_type: str = legacy['translator_type'] | ||
| self.withheld_in_countries: list[str] = legacy['withheld_in_countries'] | ||
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', []) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
legacy.get('withheld_in_countries', []) returns None when the key exists but the value is None.
build_user_data always sets 'withheld_in_countries': raw_data.get('withheld_in_countries'), so the key is present but the value can be None. dict.get(key, default) only returns the default when the key is missing, not when the value is None. Consider (legacy.get('withheld_in_countries') or []) to also handle None values.
🛡️ Proposed fix
- self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', [])
+ self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries') or []📝 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.
Suggested change
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', []) | |
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries') or [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@twikit/guest/user.py` at line 115, Update the withheld_in_countries
initialization in the user data construction flow to normalize both missing and
None values to an empty list, while preserving any provided country list. Use
the existing legacy value retrieval rather than introducing additional state.
| self.location: str = legacy['location'] | ||
| self.description: str = legacy['description'] | ||
| self.description_urls: list = legacy['entities']['description']['urls'] | ||
| self.description_urls: list = legacy['entities'].get('description', {}).get('urls', []) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Same legacy['entities'] None issue as twikit/guest/user.py.
legacy['entities'] can be None (upstream build_user_data uses raw_data.get('entities')), causing AttributeError on .get(...). This is the same incomplete defensive coding as in twikit/guest/user.py line 96.
🛡️ Proposed fix
- self.description_urls: list = legacy['entities'].get('description', {}).get('urls', [])
+ self.description_urls: list = (legacy.get('entities') or {}).get('description', {}).get('urls', [])📝 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.
Suggested change
| self.description_urls: list = legacy['entities'].get('description', {}).get('urls', []) | |
| self.description_urls: list = (legacy.get('entities') or {}).get('description', {}).get('urls', []) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@twikit/user.py` at line 102, Update the user initialization logic around
description_urls to handle legacy['entities'] being None before calling .get(),
matching the defensive handling already implemented in twikit/guest/user.py.
Preserve the existing empty URL-list fallback when entities or description data
is absent.
| self.is_translator: bool = legacy['is_translator'] | ||
| self.translator_type: str = legacy['translator_type'] | ||
| self.withheld_in_countries: list[str] = legacy['withheld_in_countries'] | ||
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', []) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Same withheld_in_countries None issue as twikit/guest/user.py.
legacy.get('withheld_in_countries', []) returns None (not []) when the key exists with a None value. Use or [] to handle both cases.
🛡️ Proposed fix
- self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', [])
+ self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries') or []📝 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.
Suggested change
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', []) | |
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries') or [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@twikit/user.py` at line 124, Update the `withheld_in_countries` assignment in
the user model to use the legacy value when present but fall back to an empty
list when it is `None`, matching the handling in `twikit/guest/user.py`.
legacy['pinned_tweet_ids_str'] is absent for accounts that have never
pinned a tweet (observed on several corporate/official accounts,
e.g. accounts used purely for announcements), raising
KeyError('pinned_tweet_ids_str') and aborting the whole tweet fetch.
Same fix as the description_urls / withheld_in_countries guards
already applied here: switched to .get(...) with an empty-list
default in both User (twikit/user.py) and the guest-mode User
(twikit/guest/user.py).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Exception: Couldn't get KEY_BYTE indices, which currently breaks every request (see Title: ClientTransaction broken as of March 18 2026 — Couldn't get KEY_BYTE indices #408, Couldn't get KEY_BYTE indices #409). X changed the webpack chunk format used to referenceondemand.s.*.jsin the homepage HTML, soON_DEMAND_FILE_REGEXno longer matches.ondemand.s, then look up that chunk's hash separately).ON_DEMAND_HASH_PATTERNto accept both single- and double-quoted hash values — the original PR only handled double quotes, which left the same failure mode for single-quoted chunk data.KeyError('urls')/KeyError('withheld_in_countries')raised byUser.__init__(bothtwikit/user.pyandtwikit/guest/user.py) when X omitsentities.description.urlsorwithheld_in_countriesfrom a user's legacy payload (observed for accounts with no links in their bio). Both fields now use.get(...)with safe defaults, matching the pattern already used for sibling optional fields likeprofile_banner_url.Test plan
Client.get_user_tweets()now succeeds end-to-end and returns real tweet content, where it previously raisedCouldn't get KEY_BYTE indicesimmediately.ON_DEMAND_HASH_PATTERNquote-handling fix with a standalone regex test against both single- and double-quoted synthetic chunk data.Summary by Sourcery
Update X client transaction parsing to restore KEY_BYTE index discovery and harden against changes in ondemand webpack chunks, while making user field handling resilient to missing legacy fields and documenting this fork as a temporary community patch.
Bug Fixes:
Documentation:
Summary by CodeRabbit