fix!: raise on request and user parsing failures (#347)

Co-authored-by: vladkens <[email protected]>

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

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"
+    }
+  ]
+}

tests/mocked-data/_issue_346_user_by_id_not_found.json

@@ -0,0 +1,5 @@
+{
+  "data": {
+    "user": {}
+  }
+}

tests/mocked-data/_issue_346_user_by_login_not_found.json

@@ -0,0 +1,3 @@
+{
+  "data": {}
+}

tests/mocked-data/_issue_346_user_suspended.json

@@ -0,0 +1,11 @@
+{
+  "data": {
+    "user": {
+      "result": {
+        "__typename": "UserUnavailable",
+        "message": "User is suspended",
+        "reason": "Suspended"
+      }
+    }
+  }
+}

tests/mocked-data/_issue_346_user_tweets_unavailable.json

@@ -0,0 +1,9 @@
+{
+  "data": {
+    "user": {
+      "result": {
+        "__typename": "UserUnavailable"
+      }
+    }
+  }
+}

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": "<html>blocked</html>",
+    "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 == []

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

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

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

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:

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):