diff --git a/readme.md b/readme.md index 7816f20..f8ab36b 100644 --- a/readme.md +++ b/readme.md @@ -118,6 +118,8 @@ api = API(raise_when_no_account=True, wait_timeout=30, wait_interval=1) `wait_timeout` limits how long to wait for a locked account, `wait_interval` controls how often the pool checks again, and `raise_when_no_account` raises `NoAccountError` instead of ending the operation. By default, twscrape waits indefinitely while active accounts are locked. +A failed request raises instead of looking like an empty result: `AbortReqError` when twscrape gives up on it (for example an HTML block page), its subclass `ApiError` when X answers with errors and no data, and `XClIdParseError` when the X client transaction ID can't be generated. A paginated method raises on the page that failed, after yielding the earlier pages. `user_by_id()` and `user_by_login()` also raise when X sends a user twscrape can't parse, so `None` means X has no such user (or no account was available and `raise_when_no_account` is off). + Search defaults to the Latest tab. Pass `kv={"product": "Top"}` or `kv={"product": "Media"}` to use another search product: ```python diff --git a/tests/mocked-data/_issue_346_deadline_exceeded_empty_user.json b/tests/mocked-data/_issue_346_deadline_exceeded_empty_user.json new file mode 100644 index 0000000..d59205b --- /dev/null +++ b/tests/mocked-data/_issue_346_deadline_exceeded_empty_user.json @@ -0,0 +1,28 @@ +{ + "data": { + "user": {} + }, + "errors": [ + { + "extensions": { + "kind": "ServiceLevel", + "name": "DeadlineExceeded", + "source": "Server" + }, + "kind": "ServiceLevel", + "locations": [ + { + "column": 5, + "line": 3 + } + ], + "message": "DeadlineExceeded: Unspecified", + "name": "DeadlineExceeded", + "path": [ + "user", + "result" + ], + "source": "Server" + } + ] +} diff --git a/tests/mocked-data/_issue_346_user_by_id_not_found.json b/tests/mocked-data/_issue_346_user_by_id_not_found.json new file mode 100644 index 0000000..155c641 --- /dev/null +++ b/tests/mocked-data/_issue_346_user_by_id_not_found.json @@ -0,0 +1,5 @@ +{ + "data": { + "user": {} + } +} diff --git a/tests/mocked-data/_issue_346_user_by_login_not_found.json b/tests/mocked-data/_issue_346_user_by_login_not_found.json new file mode 100644 index 0000000..73cd5c3 --- /dev/null +++ b/tests/mocked-data/_issue_346_user_by_login_not_found.json @@ -0,0 +1,3 @@ +{ + "data": {} +} diff --git a/tests/mocked-data/_issue_346_user_suspended.json b/tests/mocked-data/_issue_346_user_suspended.json new file mode 100644 index 0000000..a95fa80 --- /dev/null +++ b/tests/mocked-data/_issue_346_user_suspended.json @@ -0,0 +1,11 @@ +{ + "data": { + "user": { + "result": { + "__typename": "UserUnavailable", + "message": "User is suspended", + "reason": "Suspended" + } + } + } +} diff --git a/tests/mocked-data/_issue_346_user_tweets_unavailable.json b/tests/mocked-data/_issue_346_user_tweets_unavailable.json new file mode 100644 index 0000000..ac80d61 --- /dev/null +++ b/tests/mocked-data/_issue_346_user_tweets_unavailable.json @@ -0,0 +1,9 @@ +{ + "data": { + "user": { + "result": { + "__typename": "UserUnavailable" + } + } + } +} diff --git a/tests/test_api.py b/tests/test_api.py index e80b70c..0249511 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,16 +1,47 @@ +import json import os import pytest +from twscrape.account import Account from twscrape.accounts_pool import NoAccountError from twscrape.api import API +from twscrape.queue_client import AbortReqError, ApiError from twscrape.utils import gather, get_env_bool +from .mock_http import MockClient + +DATA_DIR = os.path.join(os.path.dirname(__file__), "mocked-data") + +CLOUDFLARE_BLOCK = { + "status_code": 403, + "text": "blocked", + "headers": {"content-type": "text/html", "cf-ray": "abc123"}, +} +# Seen in production: "API unknown error: 200 - ... - (-1) DeadlineExceeded: Unspecified" +DEADLINE_EXCEEDED = {"json": {"errors": [{"code": -1, "message": "DeadlineExceeded: Unspecified"}]}} + class MockedError(Exception): pass +def load(name: str) -> dict: + with open(os.path.join(DATA_DIR, f"{name}.json")) as f: + return json.load(f) + + +# Seen in production on UserByRestId: the error comes with an empty `user` +DEADLINE_EXCEEDED_EMPTY_USER = {"json": load("_issue_346_deadline_exceeded_empty_user")} + + +@pytest.fixture +def api_http(api_mock: API, monkeypatch): + mock = MockClient() + monkeypatch.setattr(Account, "make_client", lambda self, proxy=None: mock) + return api_mock, mock + + GQL_GEN = [ "search", "tweet_replies", @@ -86,3 +117,71 @@ async def test_raise_when_no_account(api_mock: API): del os.environ["TWS_RAISE_WHEN_NO_ACCOUNT"] assert get_env_bool("TWS_RAISE_WHEN_NO_ACCOUNT") is False + + +# https://github.com/vladkens/twscrape/issues/346 +# A failed request must raise; None (or an empty generator) is kept for X's own +# "no data" answers, recorded from X in the _issue_346_*.json files. + + +@pytest.mark.parametrize( + "method,arg,data", + [ + ("user_by_id", 1, "_issue_346_user_by_id_not_found"), + ("user_by_login", "coalition_peaks", "_issue_346_user_by_login_not_found"), + ("user_by_id", 17161988, "_issue_346_user_suspended"), + ], +) +async def test_user_lookup_returns_none_when_x_has_no_user(api_http, method, arg, data): + api, mock = api_http + mock.add_response(json=load(data)) + + assert await getattr(api, method)(arg) is None + + +@pytest.mark.parametrize( + "response,error", + [ + (CLOUDFLARE_BLOCK, AbortReqError), + (DEADLINE_EXCEEDED, ApiError), + (DEADLINE_EXCEEDED_EMPTY_USER, ApiError), + ], +) +@pytest.mark.parametrize("method,arg", [("user_by_id", 123), ("user_by_login", "someone")]) +async def test_user_lookup_raises_when_request_fails(api_http, method, arg, response, error): + api, mock = api_http + mock.add_response(**response) + + with pytest.raises(error): + await getattr(api, method)(arg) + + +async def test_user_tweets_of_unavailable_user_ends_without_error(api_http): + # Protected or suspended account: X answers with UserUnavailable and no timeline + api, mock = api_http + mock.add_response(json=load("_issue_346_user_tweets_unavailable")) + + assert await gather(api.user_tweets(123)) == [] + + +@pytest.mark.parametrize( + "response,error", + [ + (CLOUDFLARE_BLOCK, AbortReqError), + (DEADLINE_EXCEEDED, ApiError), + (DEADLINE_EXCEEDED_EMPTY_USER, ApiError), + ], +) +async def test_user_tweets_raises_when_page_fails_mid_pagination(api_http, response, error): + api, mock = api_http + mock.add_response(json=load("raw_user_tweets")) # has a Bottom cursor + mock.add_response(**response) + + tweets = [] + with pytest.raises(error): + async for tweet in api.user_tweets(123): + tweets.append(tweet) + + # the caller keeps what was yielded before the failure + assert len(tweets) == 21 + assert mock._queue == [] diff --git a/tests/test_parser.py b/tests/test_parser.py index 550ebf4..7399c34 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -825,6 +825,30 @@ def test_partial_only_user_keeps_user_lookup_single(): assert doc.id == expected.id +def test_user_lookup_parse_failure_raises(): + # https://github.com/vladkens/twscrape/issues/346 + # A user X sent but that can't be parsed must not read as "no user". Here the + # looked-up user lacks its name, like the partial users of #341 and #342. + raw = fake_rep("raw_user_by_id").json() + del raw["data"]["user"]["result"]["core"]["name"] + + with pytest.raises(KeyError, match="name"): + parse_user(raw) + + +@pytest.mark.parametrize("missing", ["screen_name", "core"]) +def test_user_lookup_normalization_failure_raises(missing): + raw = fake_rep("raw_user_by_id").json() + user = raw["data"]["user"]["result"] + if missing == "core": + del user["core"] + else: + del user["core"][missing] + + with pytest.raises(ValueError, match="Failed to parse user"): + parse_user(raw) + + def test_bot_reply_to_partial_only_operator(): # Two own retweets from production UserTweets pages (@NatMus_SWE, @betaworks). Each # retweets a bot's reply to its operator, whose "Automated by @operator" label is the diff --git a/tests/test_queue_client.py b/tests/test_queue_client.py index d591406..39a49c8 100644 --- a/tests/test_queue_client.py +++ b/tests/test_queue_client.py @@ -1,4 +1,5 @@ import json +import pickle from collections import OrderedDict from contextlib import aclosing @@ -9,6 +10,8 @@ from twscrape.account import Account from twscrape.accounts_pool import AccountsPool from twscrape.http import ConnectError, HttpMethod, NetworkError, Response from twscrape.queue_client import ( + AbortReqError, + ApiError, GqlFeaturesOutdatedError, QueueClient, ReqParams, @@ -476,7 +479,8 @@ async def test_403_no_errors_marks_account_inactive(client_fixture: CF): # --- Cloudflare / HTML block --- -async def test_cloudflare_block_returns_none(client_fixture: CF): +async def test_cloudflare_block_raises(client_fixture: CF): + # https://github.com/vladkens/twscrape/issues/346 pool, client, mock = client_fixture await client.__aenter__() @@ -486,13 +490,13 @@ async def test_cloudflare_block_returns_none(client_fixture: CF): headers={"content-type": "text/html", "cf-ray": "abc123"}, ) - rep = await client.get(URL) - assert rep is None + with pytest.raises(AbortReqError, match="Blocked by Cloudflare: 403"): + await client.get(URL) await client.__aexit__(None, None, None) -async def test_html_block_without_cf_returns_none(client_fixture: CF): +async def test_html_block_without_cf_raises(client_fixture: CF): pool, client, mock = client_fixture await client.__aenter__() @@ -502,8 +506,8 @@ async def test_html_block_without_cf_returns_none(client_fixture: CF): headers={"content-type": "text/html"}, ) - rep = await client.get(URL) - assert rep is None + with pytest.raises(AbortReqError, match="Blocked by HTML: 503"): + await client.get(URL) await client.__aexit__(None, None, None) @@ -528,14 +532,15 @@ async def test_131_with_user_data_continues(client_fixture: CF): await client.__aexit__(None, None, None) -async def test_131_without_user_data_continues(client_fixture: CF): +async def test_131_without_user_data_raises(client_fixture: CF): pool, client, mock = client_fixture await client.__aenter__() mock.add_response(json={"errors": [{"code": 131, "message": "Dependency: Internal error"}]}) - rep = await client.get(URL) - assert rep is not None + with pytest.raises(ApiError) as exc: + await client.get(URL) + assert exc.value.errors == ["(131) Dependency: Internal error"] await client.__aexit__(None, None, None) @@ -671,7 +676,7 @@ async def test_missing_status_error_ignored(client_fixture: CF): await client.__aexit__(None, None, None) -async def test_authorization_error_200_ignored(client_fixture: CF): +async def test_authorization_error_200_raises(client_fixture: CF): pool, client, mock = client_fixture await client.__aenter__() @@ -679,12 +684,35 @@ async def test_authorization_error_200_ignored(client_fixture: CF): json={"errors": [{"code": -1, "message": "Authorization: Denied by unknown rule"}]} ) - rep = await client.get(URL) - assert rep is not None + with pytest.raises(ApiError, match="Authorization: Denied by unknown rule"): + await client.get(URL) await client.__aexit__(None, None, None) +async def test_deadline_exceeded_without_data_raises(client_fixture: CF): + # Seen in production: "API unknown error: 200 - ... - (-1) DeadlineExceeded: Unspecified" + pool, client, mock = client_fixture + await client.__aenter__() + + mock.add_response(json={"errors": [{"code": -1, "message": "DeadlineExceeded: Unspecified"}]}) + + with pytest.raises(ApiError) as exc: + await client.get(URL) + assert exc.value.errors == ["(-1) DeadlineExceeded: Unspecified"] + assert "SearchTimeline" in str(exc.value) + + await client.__aexit__(None, None, None) + + +def test_api_error_pickles(): + # Process pools send exceptions back to the parent with pickle + err = pickle.loads(pickle.dumps(ApiError("API error: x", ["(-1) LoadShed: Unspecified"]))) + + assert str(err) == "API error: x" + assert err.errors == ["(-1) LoadShed: Unspecified"] + + async def test_unknown_error_warned_once(client_fixture: CF, monkeypatch): _pool, client, mock = client_fixture logs = [] @@ -696,8 +724,8 @@ async def test_unknown_error_warned_once(client_fixture: CF, monkeypatch): for _ in range(2): mock.add_response(json={"errors": [{"code": 999, "message": "Some unfamiliar error"}]}) - rep = await client.get(URL) - assert rep is not None + with pytest.raises(ApiError): + await client.get(URL) assert [level for level, _msg in logs] == ["WARNING"] @@ -718,8 +746,8 @@ async def test_unknown_error_statuses_are_logged_separately(client_fixture: CF, status_code=status_code, json={"errors": [{"code": 999, "message": "Some unfamiliar error"}]}, ) - rep = await client.get(URL) - assert rep is not None + with pytest.raises(ApiError): + await client.get(URL) assert [level for level, _msg in logs] == ["WARNING", "WARNING"] await client.__aexit__(None, None, None) @@ -747,6 +775,25 @@ async def test_loadshed_without_data_retries_same_account(client_fixture: CF, mo await client.__aexit__(None, None, None) +async def test_loadshed_without_data_raises_when_retries_exhausted(client_fixture: CF, monkeypatch): + _pool, client, mock = client_fixture + + async def fake_sleep(secs): + pass + + monkeypatch.setattr("twscrape.queue_client.asyncio.sleep", fake_sleep) + await client.__aenter__() + + for _ in range(3): + mock.add_response(json={"errors": [{"code": -1, "message": "LoadShed: Unspecified"}]}) + + with pytest.raises(ApiError, match="LoadShed"): + await client.get(URL) + assert mock._queue == [] + + await client.__aexit__(None, None, None) + + async def test_api_error_with_data_is_throttled(client_fixture: CF, monkeypatch): _pool, client, mock = client_fixture logs = [] @@ -777,8 +824,10 @@ async def test_api_error_with_data_is_throttled(client_fixture: CF, monkeypatch) await client.__aexit__(None, None, None) -@pytest.mark.parametrize("data", [None, {}, {"user": None}]) -async def test_api_error_without_useful_data_is_warned(client_fixture: CF, monkeypatch, data): +@pytest.mark.parametrize("data", [None, {}, {"user": None}, {"user": {}}]) +async def test_api_error_without_useful_data_is_warned_and_raises( + client_fixture: CF, monkeypatch, data +): _pool, client, mock = client_fixture logs = [] monkeypatch.setattr(queue_client_module.LogOnce, "seen", OrderedDict()) @@ -793,9 +842,9 @@ async def test_api_error_without_useful_data_is_warned(client_fixture: CF, monke "errors": [{"code": -1, "message": "Dependency: Unspecified"}], } ) - rep = await client.get(URL) + with pytest.raises(ApiError): + await client.get(URL) - assert rep is not None assert [level for level, _msg in logs] == ["WARNING"] await client.__aexit__(None, None, None) @@ -870,9 +919,11 @@ async def test_404_retries_exhaust_and_abort(client_fixture: CF): mock.add_response(status_code=404, json={}) mock.add_response(status_code=404, json={}) - with patch("twscrape.queue_client.asyncio.sleep"): - rep = await client.get(URL) - assert rep is None + with ( + patch("twscrape.queue_client.asyncio.sleep"), + pytest.raises(AbortReqError, match="XClIdGen"), + ): + await client.get(URL) await client.__aexit__(None, None, None) @@ -1080,9 +1131,9 @@ async def test_xclid_parse_error_aborts_without_account_state_change( monkeypatch.setattr("twscrape.queue_client.logger.error", messages.append) client = QueueClient(pool_mock, "SearchTimeline") - rep = await client.get(URL) + with pytest.raises(XClIdParseError, match="Signing script not found"): + await client.get(URL) - assert rep is None user1 = await pool_mock.get("user1") assert user1.active is True assert "SearchTimeline" not in user1.locks diff --git a/twscrape/__init__.py b/twscrape/__init__.py index da4594f..366b132 100644 --- a/twscrape/__init__.py +++ b/twscrape/__init__.py @@ -5,5 +5,6 @@ from .api import API from .http import ConnectError, HttpError, HttpStatusError, NetworkError, Response from .logger import set_log_level from .models import * # noqa: F403 -from .queue_client import GqlFeaturesOutdatedError +from .queue_client import AbortReqError, ApiError, GqlFeaturesOutdatedError from .utils import gather +from .xclid import XClIdParseError diff --git a/twscrape/models.py b/twscrape/models.py index dfe283c..6d58c9d 100644 --- a/twscrape/models.py +++ b/twscrape/models.py @@ -1234,6 +1234,7 @@ def _parse_items( limit: int = -1, seen_ids: set[int | str | None] | None = None, accept: Callable[[ParsedItem], bool] | None = None, + errors: list[Exception] | None = None, ) -> Generator[ParsedItem, None, None]: key = kind if kind == "trends" else f"{kind}s" @@ -1266,6 +1267,8 @@ def _parse_items( yield tmp except Exception as e: _write_dump(kind, e, x, obj) + if errors is not None: + errors.append(e) continue @@ -1285,14 +1288,22 @@ def parse_tweet(rep: Response, twid: int) -> Tweet | None: def parse_user(rep: Response) -> User | None: - try: - docs = list(parse_users(rep)) - if len(docs) == 1: - return docs[0] - return None - except Exception as e: - logger.error(f"Failed to parse user - {type(e)}:\n{traceback.format_exc()}") - return None + # Propagate parsing failures instead of treating them as missing users: + # https://github.com/vladkens/twscrape/issues/346 + errors: list[Exception] = [] + docs = list(_parse_items(rep, "user", User.parse, errors=errors)) + if len(docs) == 1: + return docs[0] + if not docs and errors: + raise errors[0] + if not docs: + res = rep if isinstance(rep, dict) else rep.json() + user = get_or(res, "data.user", {}) or {} + user = user.get("result") + if isinstance(user, dict) and user.get("__typename") == "User": + raise ValueError(f"Failed to parse user {user.get('rest_id', '')}") + + return None def parse_trend(rep: Response) -> Trend | None: diff --git a/twscrape/queue_client.py b/twscrape/queue_client.py index 08a5870..fce5805 100644 --- a/twscrape/queue_client.py +++ b/twscrape/queue_client.py @@ -37,6 +37,15 @@ class GqlFeaturesOutdatedError(AbortReqError): """GQL_FEATURES in api.py no longer matches the X API and self-healing did not resolve it.""" +class ApiError(AbortReqError): + """X answered with errors and without data.""" + + # errors is optional so that pickle, which calls ApiError(msg), can rebuild it + def __init__(self, msg: str, errors: list[str] | None = None): + super().__init__(msg) + self.errors = errors or [] + + class FailKind(Enum): TRANSPORT = auto() LOADSHED = auto() @@ -109,7 +118,7 @@ class Ctx: await asyncio.sleep(1) raise AbortReqError( - "Faield to get XClIdGen. See: https://github.com/vladkens/twscrape/issues/248" + "Failed to get XClIdGen. See: https://github.com/vladkens/twscrape/issues/248" ) @@ -124,12 +133,14 @@ def req_id(rep: Response): def has_data(rep: Response, res: Any) -> bool: - """Return True for successful responses with at least one non-null data field.""" + """Return True for successful responses with at least one non-empty data field.""" if rep.status_code != 200 or not isinstance(res, dict): return False + # An empty field is no data: on a timeout X sends {"data": {"user": {}}} + # with DeadlineExceeded at path user.result. data = res.get("data") - return isinstance(data, dict) and any(value is not None for value in data.values()) + return isinstance(data, dict) and any(value not in (None, {}, []) for value in data.values()) def has_error(errors: list[str], prefix: str) -> bool: @@ -256,8 +267,9 @@ class QueueClient: if "text/html" in rep.headers.get("content-type", "") and rep.status_code >= 400: src = "Cloudflare" if "cf-ray" in rep.headers else "HTML" - logger.warning(f"Blocked by {src}: {rep.status_code} - {req_id(rep)}") - raise AbortReqError() + msg = f"Blocked by {src}: {rep.status_code} - {req_id(rep)}" + logger.warning(msg) + raise AbortReqError(msg) try: res = rep.json() @@ -328,10 +340,12 @@ class QueueClient: ctx = self.ctx if ctx is not None and await ctx.retry(FailKind.LOADSHED): raise HandledError() - return + else: + LogOnce.once(log_key, "WARNING", f"API unknown error: {summary_log}") - LogOnce.once(log_key, "WARNING", f"API unknown error: {summary_log}") - return + # Without data the parser would find nothing, and the caller would read + # a failure as an empty result. + raise ApiError(f"API error: {summary_log}", errors) try: rep.raise_for_status() @@ -394,8 +408,9 @@ class QueueClient: ) continue except AbortReqError: - # abort all queries - return + # abort all queries; raise rather than return None, which callers + # can't tell from "no data" + raise except HandledError: # retry with new account continue @@ -409,7 +424,7 @@ class QueueClient: "Report: https://github.com/vladkens/twscrape/issues" ) await self._close_ctx() - return None + raise except (NetworkError, ConnectError) as e: # transport failed, retry same account with backoff, then cool it down and rotate if await ctx.retry(FailKind.TRANSPORT):