fix(bird_x): retry subprocess on non-JSON stdout (HTML interstitial)
Twitter's edge intermittently serves an HTML anti-bot interstitial in
place of JSON when the bird-search subprocess hits a per-query rate
limit. Before this fix, that response made json.loads raise
JSONDecodeError and _run_bird_search() returned {"error": ..., "items":
[]} with the parsed exception message — silent-empty against an
orchestrator that has no way to distinguish "Twitter served HTML; retry
likely succeeds" from "no tweets matched the query."
Surfaced during a community-signal pass where a Karpathy-LLM-wiki
subquery returned zero X items, while a second identical run a few
seconds later returned full results.
Fix:
- Extract the subprocess invocation into _invoke_bird_subprocess() so
the retry loop can call it multiple times cleanly. Returns
(result, terminal_error) — terminal_error is non-None for
unrecoverable cases (subprocess timeout, spawn failure) that should
NOT be retried.
- In _run_bird_search(), wrap the json.loads parse in a retry loop
bounded by MAX_JSON_DECODE_RETRIES (=2) with JSON_DECODE_RETRY_DELAY
(=5s) between attempts.
- On non-JSON stdout, log a diagnostic that names the shape
(`looks_html`, first-80-chars stdout preview, attempt counter) so
silent-empty failures become legible in logs.
- On retry exhaustion, return an error dict whose message explicitly
names "anti-bot interstitial" as the likely cause, distinguishing
this failure from a genuine no-results case.
Subprocess timeout, spawn failure, and non-zero return-code paths are
unchanged — those are terminal and don't retry.
Tests:
- Verifies HTML-then-JSON returns success on attempt 2.
- Verifies all-HTML returns the diagnostic error dict mentioning the
anti-bot interstitial cause.
- Verifies subprocess timeout is NOT retried.
All 12 bird_x tests pass (9 baseline + 3 new).
This commit is contained in:
committed by
Trevin Chow
parent
602de1ebda
commit
a717dd2b2c
@@ -9,6 +9,7 @@ import json
|
|||||||
import os
|
import os
|
||||||
import shutil
|
import shutil
|
||||||
import sys
|
import sys
|
||||||
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from . import http, log, subproc
|
from . import http, log, subproc
|
||||||
@@ -17,6 +18,11 @@ from typing import Any, Dict, List, Optional, Tuple
|
|||||||
|
|
||||||
from .relevance import token_overlap_relevance as _compute_relevance
|
from .relevance import token_overlap_relevance as _compute_relevance
|
||||||
|
|
||||||
|
# How many times to retry the bird-search subprocess when stdout is non-JSON
|
||||||
|
# (typically an HTML anti-bot interstitial from Twitter's edge).
|
||||||
|
MAX_JSON_DECODE_RETRIES = 2
|
||||||
|
JSON_DECODE_RETRY_DELAY = 5.0 # seconds between retry attempts
|
||||||
|
|
||||||
|
|
||||||
def _first_of(*values):
|
def _first_of(*values):
|
||||||
"""Return first value that is not None."""
|
"""Return first value that is not None."""
|
||||||
@@ -148,16 +154,14 @@ def get_bird_status() -> Dict[str, Any]:
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def _run_bird_search(query: str, count: int, timeout: int) -> Dict[str, Any]:
|
def _invoke_bird_subprocess(query: str, count: int, timeout: int):
|
||||||
"""Run a search using the vendored bird-search.mjs module.
|
"""Invoke the vendored bird-search.mjs subprocess once.
|
||||||
|
|
||||||
Args:
|
Returns (result, error_dict). If error_dict is non-None, treat it as the
|
||||||
query: Full search query string (including since: filter)
|
final result and do not retry — those errors are terminal (timeout,
|
||||||
count: Number of results to request
|
spawn failure). If error_dict is None, the subprocess ran to completion
|
||||||
timeout: Timeout in seconds
|
and `result` is the SubprocResult; the caller decides whether to retry
|
||||||
|
based on the result.stdout content.
|
||||||
Returns:
|
|
||||||
Raw Bird JSON response or error dict.
|
|
||||||
"""
|
"""
|
||||||
cmd = [
|
cmd = [
|
||||||
"node", str(_BIRD_SEARCH_MJS),
|
"node", str(_BIRD_SEARCH_MJS),
|
||||||
@@ -184,9 +188,9 @@ def _run_bird_search(query: str, count: int, timeout: int) -> Dict[str, Any]:
|
|||||||
on_pid=_register,
|
on_pid=_register,
|
||||||
)
|
)
|
||||||
except subproc.SubprocTimeout:
|
except subproc.SubprocTimeout:
|
||||||
return {"error": f"Search timed out after {timeout}s", "items": []}
|
return None, {"error": f"Search timed out after {timeout}s", "items": []}
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return {"error": str(e), "items": []}
|
return None, {"error": str(e), "items": []}
|
||||||
finally:
|
finally:
|
||||||
if pid_holder:
|
if pid_holder:
|
||||||
try:
|
try:
|
||||||
@@ -195,22 +199,80 @@ def _run_bird_search(query: str, count: int, timeout: int) -> Dict[str, Any]:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
if result.returncode != 0:
|
return result, None
|
||||||
error = result.stderr.strip() or "Bird search failed"
|
|
||||||
return {"error": error, "items": []}
|
|
||||||
|
|
||||||
output = result.stdout.strip()
|
|
||||||
if not output:
|
|
||||||
return {"items": []}
|
|
||||||
|
|
||||||
try:
|
def _run_bird_search(query: str, count: int, timeout: int) -> Dict[str, Any]:
|
||||||
parsed = json.loads(output)
|
"""Run a search using the vendored bird-search.mjs module.
|
||||||
except json.JSONDecodeError as e:
|
|
||||||
return {"error": f"Invalid JSON response: {e}", "items": []}
|
|
||||||
|
|
||||||
if isinstance(parsed, list):
|
Retries the subprocess on JSON-decode failure (typically a Twitter
|
||||||
return {"items": parsed}
|
anti-bot HTML interstitial in stdout) up to MAX_JSON_DECODE_RETRIES
|
||||||
return parsed
|
times with JSON_DECODE_RETRY_DELAY seconds between attempts. Terminal
|
||||||
|
errors (subprocess timeout, non-zero return code) are returned
|
||||||
|
immediately without retry.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
query: Full search query string (including since: filter)
|
||||||
|
count: Number of results to request
|
||||||
|
timeout: Timeout in seconds (per attempt)
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
Raw Bird JSON response or error dict.
|
||||||
|
"""
|
||||||
|
last_decode_error: Optional[str] = None
|
||||||
|
|
||||||
|
for attempt in range(MAX_JSON_DECODE_RETRIES):
|
||||||
|
result, terminal_error = _invoke_bird_subprocess(query, count, timeout)
|
||||||
|
if terminal_error is not None:
|
||||||
|
return terminal_error
|
||||||
|
|
||||||
|
if result.returncode != 0:
|
||||||
|
error = result.stderr.strip() or "Bird search failed"
|
||||||
|
return {"error": error, "items": []}
|
||||||
|
|
||||||
|
output = result.stdout.strip()
|
||||||
|
if not output:
|
||||||
|
return {"items": []}
|
||||||
|
|
||||||
|
try:
|
||||||
|
parsed = json.loads(output)
|
||||||
|
except json.JSONDecodeError as e:
|
||||||
|
# Twitter's edge sometimes serves an HTML anti-bot interstitial
|
||||||
|
# in place of JSON. Tag the failure shape so it's distinguishable
|
||||||
|
# from "no results" in logs, then retry the subprocess.
|
||||||
|
looks_html = output.lstrip().lower().startswith(("<!doctype", "<html", "<"))
|
||||||
|
attempt_num = attempt + 1
|
||||||
|
log_msg = (
|
||||||
|
f"Bird search returned non-JSON stdout "
|
||||||
|
f"(looks_html={looks_html}, attempt {attempt_num}/{MAX_JSON_DECODE_RETRIES}, "
|
||||||
|
f"first 80 chars: {output[:80]!r})"
|
||||||
|
)
|
||||||
|
last_decode_error = str(e)
|
||||||
|
if attempt_num < MAX_JSON_DECODE_RETRIES:
|
||||||
|
log.source_log(
|
||||||
|
"X/bird",
|
||||||
|
f"{log_msg}; retrying in {JSON_DECODE_RETRY_DELAY:.0f}s",
|
||||||
|
)
|
||||||
|
time.sleep(JSON_DECODE_RETRY_DELAY)
|
||||||
|
continue
|
||||||
|
log.source_log("X/bird", log_msg)
|
||||||
|
return {
|
||||||
|
"error": (
|
||||||
|
f"Invalid JSON response after {MAX_JSON_DECODE_RETRIES} attempts "
|
||||||
|
f"(likely Twitter anti-bot interstitial): {e}"
|
||||||
|
),
|
||||||
|
"items": [],
|
||||||
|
}
|
||||||
|
|
||||||
|
if isinstance(parsed, list):
|
||||||
|
return {"items": parsed}
|
||||||
|
return parsed
|
||||||
|
|
||||||
|
# Defensive fallthrough — loop should always return above.
|
||||||
|
return {
|
||||||
|
"error": f"Bird search exhausted retries: {last_decode_error}",
|
||||||
|
"items": [],
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def search_x(
|
def search_x(
|
||||||
|
|||||||
@@ -232,5 +232,79 @@ class TestVendoredBirdRuntime(unittest.TestCase):
|
|||||||
self.assertEqual(5, items[0]["engagement"]["likes"])
|
self.assertEqual(5, items[0]["engagement"]["likes"])
|
||||||
|
|
||||||
|
|
||||||
|
class TestRunBirdSearchJsonDecodeRetry(unittest.TestCase):
|
||||||
|
"""When bird-search returns non-JSON stdout, retry the subprocess.
|
||||||
|
|
||||||
|
Twitter's edge sometimes serves an HTML anti-bot interstitial in place of
|
||||||
|
JSON. Before this fix, that response made json.loads raise JSONDecodeError
|
||||||
|
and the function returned {"items": []} with no diagnostic — silent-empty
|
||||||
|
against an orchestrator that can't distinguish "Twitter blocked us" from
|
||||||
|
"no tweets matched the query."
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _make_result(self, stdout: str, stderr: str = "", returncode: int = 0):
|
||||||
|
from lib.subproc import SubprocResult
|
||||||
|
return SubprocResult(returncode=returncode, stdout=stdout, stderr=stderr)
|
||||||
|
|
||||||
|
def test_retries_subprocess_on_html_interstitial_then_succeeds(self):
|
||||||
|
"""First subprocess attempt returns HTML; second returns JSON → success."""
|
||||||
|
from unittest import mock
|
||||||
|
from lib import bird_x
|
||||||
|
|
||||||
|
html_interstitial = "<!DOCTYPE html><html><body>Rate limited</body></html>"
|
||||||
|
json_success = '[{"id": "1", "text": "tweet"}]'
|
||||||
|
|
||||||
|
results = [
|
||||||
|
(self._make_result(stdout=html_interstitial), None),
|
||||||
|
(self._make_result(stdout=json_success), None),
|
||||||
|
]
|
||||||
|
|
||||||
|
with mock.patch.object(bird_x, "_invoke_bird_subprocess", side_effect=results), \
|
||||||
|
mock.patch.object(bird_x.time, "sleep") as mock_sleep:
|
||||||
|
response = bird_x._run_bird_search("test", count=10, timeout=30)
|
||||||
|
|
||||||
|
self.assertNotIn("error", response)
|
||||||
|
self.assertEqual(response["items"], [{"id": "1", "text": "tweet"}])
|
||||||
|
# Should have slept between the failed first attempt and the retry.
|
||||||
|
mock_sleep.assert_called_once_with(bird_x.JSON_DECODE_RETRY_DELAY)
|
||||||
|
|
||||||
|
def test_returns_error_after_all_retries_exhausted(self):
|
||||||
|
"""All attempts return HTML → error dict with diagnostic + items=[]."""
|
||||||
|
from unittest import mock
|
||||||
|
from lib import bird_x
|
||||||
|
|
||||||
|
html_interstitial = "<!DOCTYPE html><html>blocked</html>"
|
||||||
|
results = [
|
||||||
|
(self._make_result(stdout=html_interstitial), None),
|
||||||
|
(self._make_result(stdout=html_interstitial), None),
|
||||||
|
]
|
||||||
|
|
||||||
|
with mock.patch.object(bird_x, "_invoke_bird_subprocess", side_effect=results), \
|
||||||
|
mock.patch.object(bird_x.time, "sleep"):
|
||||||
|
response = bird_x._run_bird_search("test", count=10, timeout=30)
|
||||||
|
|
||||||
|
self.assertIn("error", response)
|
||||||
|
self.assertIn("Invalid JSON response", response["error"])
|
||||||
|
# Diagnostic message names the anti-bot interstitial so it's
|
||||||
|
# distinguishable from a genuine no-results case in logs.
|
||||||
|
self.assertIn("anti-bot interstitial", response["error"].lower())
|
||||||
|
self.assertEqual(response["items"], [])
|
||||||
|
|
||||||
|
def test_terminal_subprocess_error_is_not_retried(self):
|
||||||
|
"""Subprocess timeout / spawn failure → terminal error, no retry."""
|
||||||
|
from unittest import mock
|
||||||
|
from lib import bird_x
|
||||||
|
|
||||||
|
timeout_error = {"error": "Search timed out after 30s", "items": []}
|
||||||
|
results = [(None, timeout_error)]
|
||||||
|
|
||||||
|
with mock.patch.object(bird_x, "_invoke_bird_subprocess", side_effect=results), \
|
||||||
|
mock.patch.object(bird_x.time, "sleep") as mock_sleep:
|
||||||
|
response = bird_x._run_bird_search("test", count=10, timeout=30)
|
||||||
|
|
||||||
|
self.assertEqual(response, timeout_error)
|
||||||
|
mock_sleep.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user