diff --git a/CHANGELOG.md b/CHANGELOG.md index 38ae7e8..7f3bdbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ # Changelog +## Unreleased + +- Retry a TibiaWiki API request that gets no response within 60 seconds, a response cut short mid-body, a 429 or 5xx + status, or a body that is not JSON, up to 5 attempts, waiting 5, 15, 30 and 60 seconds between them. Any other + status, an API error or a fifth failure stops the run with a `WikiApiError` that names the status and the request. + Before, articles and category members had no timeout and stopped at the first bad response, and image info retried + a failed request forever. + ## 9.0.0+tibiash.7 - Read loot ranges with commas as thousands separators, like `1,000-2,000`, as 1000 to 2000 instead of 0 to 2. This diff --git a/tests/tests_wikiapi.py b/tests/tests_wikiapi.py index 639061a..95ab282 100644 --- a/tests/tests_wikiapi.py +++ b/tests/tests_wikiapi.py @@ -1,21 +1,80 @@ +# Changed by tibia.sh in 2026. See "About this copy" in README.md. +import json import unittest -from unittest.mock import MagicMock +from unittest.mock import Mock, call + +import requests -import tibiawikisql.api from tests import load_resource from tibiawikisql.api import Article, WikiClient, WikiEntry, Image +from tibiawikisql.errors import WikiApiError -class TestWikiApi(unittest.TestCase): +def response(status_code: int, text: str) -> Mock: + """A stand-in for a `requests.Response` with only an explicit status and text.""" + return Mock(spec=["status_code", "text"], status_code=status_code, text=text) + + +def revisions_body(titles: list[str]) -> str: + """A revisions query body holding one article per title.""" + pages = { + str(page_id): { + "pageid": page_id, + "ns": 0, + "title": title, + "revisions": [{"timestamp": "2017-04-26T20:04:19Z", "*": f"Content of {title}"}], + } + for page_id, title in enumerate(titles, start=1) + } + return json.dumps({"query": {"pages": pages}}) +def category_body(titles: list[str], cmcontinue: str | None = None) -> str: + """A category members query body holding one page per title, continued from `cmcontinue` if given.""" + members = [ + {"pageid": page_id, "ns": 0, "title": title, "sortkeyprefix": "", "timestamp": "2017-04-26T20:04:19Z"} + for page_id, title in enumerate(titles, start=1) + ] + body = {"query": {"categorymembers": members}} + if cmcontinue is not None: + body["continue"] = {"cmcontinue": cmcontinue, "continue": "-||"} + return json.dumps(body) + + +class TestWikiApi(unittest.TestCase): + def setUp(self): - self.wiki_client = WikiClient() + self.sleep = Mock() + self.wiki_client = WikiClient(sleep=self.sleep) + self.session = Mock() + self.wiki_client.session = self.session + self.sent_params = [] + + def respond(self, *outcomes): + """Make the session answer each request with the next outcome, a response or an exception to raise. + + A copy of each request's params is kept in `sent_params`, since the client reuses one dict across requests. + """ + remaining = iter(outcomes) + + def get(*args, **kwargs): + self.sent_params.append(dict(kwargs["params"])) + outcome = next(remaining) + if isinstance(outcome, Exception): + raise outcome + return outcome + + self.session.get.side_effect = get + + def assert_requests(self, count: int): + self.assertEqual(self.session.get.call_count, count) + for request in self.session.get.call_args_list: + self.assertEqual(request.args, (WikiClient.ENDPOINT,)) + self.assertEqual(request.kwargs["timeout"], 60) def test_category_functions(self): json_response = load_resource("response_category_without_continue.json") - tibiawikisql.api.requests.Session.get = MagicMock() - tibiawikisql.api.requests.Session.get.return_value.text = json_response + self.respond(*[response(200, json_response)] * 3) members = list(self.wiki_client.get_category_members("Spells")) self.assertIsInstance(members[0], WikiEntry) self.assertEqual(len(members), 8) @@ -26,11 +85,12 @@ def test_category_functions(self): titles = list(self.wiki_client.get_category_members_titles("Spells")) self.assertIsInstance(titles[0], str) self.assertEqual(len(titles), 8) + self.assert_requests(3) + self.sleep.assert_not_called() def test_article_functions(self): json_response = load_resource("response_revisions.json") - tibiawikisql.api.requests.Session.get = MagicMock() - tibiawikisql.api.requests.Session.get.return_value.text = json_response + self.respond(*[response(200, json_response)] * 2) # Response is mocked, so this doesn't affect the output, but this matches the order in the mocked response. titles = ["Golden Armor", "Golden Shield"] articles = list(self.wiki_client.get_articles(titles)) @@ -41,12 +101,12 @@ def test_article_functions(self): article = self.wiki_client.get_article(titles[0]) self.assertIsInstance(article, Article) self.assertEqual(article.title, titles[0]) + self.assert_requests(2) + self.sleep.assert_not_called() def test_image_functions(self): json_response = load_resource("response_image_info.json") - tibiawikisql.api.requests.Session.get = MagicMock() - tibiawikisql.api.requests.Session.get.return_value.text = json_response - tibiawikisql.api.requests.Session.get.return_value.status_code = 200 + self.respond(*[response(200, json_response)] * 2) # Response is mocked, so this doesn't affect the output, but this matches the order in the mocked response. titles = ["Golden Armor.gif", "Golden Shield.gif"] images = list(self.wiki_client.get_images_info(titles)) @@ -59,3 +119,197 @@ def test_image_functions(self): self.assertEqual(image.file_name, titles[0]) self.assertEqual(image.extension, ".gif") self.assertEqual(image.clean_name, "Golden Armor") + self.assert_requests(2) + self.sleep.assert_not_called() + + def test_empty_body_is_retried(self): + self.respond(response(200, ""), response(200, load_resource("response_revisions.json"))) + + articles = list(self.wiki_client.get_articles(["Golden Armor", "Golden Shield"])) + + self.assertEqual(articles[0].title, "Golden Armor") + self.assertIsNone(articles[1]) + self.assert_requests(2) + self.assertEqual(self.sleep.call_args_list, [call(5)]) + + def test_server_error_is_retried(self): + self.respond(response(502, "Bad Gateway"), response(200, load_resource("response_revisions.json"))) + + article = self.wiki_client.get_article("Golden Armor") + + self.assertEqual(article.title, "Golden Armor") + self.assert_requests(2) + self.assertEqual(self.sleep.call_args_list, [call(5)]) + + def test_too_many_requests_is_retried(self): + self.respond(response(429, "Too Many Requests"), response(200, load_resource("response_revisions.json"))) + + article = self.wiki_client.get_article("Golden Armor") + + self.assertEqual(article.title, "Golden Armor") + self.assert_requests(2) + self.assertEqual(self.sleep.call_args_list, [call(5)]) + + def test_connection_error_is_retried(self): + self.respond( + requests.ConnectionError("connection reset"), + response(200, load_resource("response_category_without_continue.json")), + ) + + members = list(self.wiki_client.get_category_members("Spells")) + + self.assertEqual(len(members), 8) + self.assert_requests(2) + self.assertEqual(self.sleep.call_args_list, [call(5)]) + + def test_category_members_follow_cmcontinue(self): + self.respond( + response(200, category_body(["Spell 1", "Spell 2"], cmcontinue="page|5350454c4c2033|123")), + response(200, category_body(["Spell 3"])), + ) + + titles = [member.title for member in self.wiki_client.get_category_members("Spells")] + + self.assertEqual(titles, ["Spell 1", "Spell 2", "Spell 3"]) + self.assert_requests(2) + self.assertIsNone(self.sent_params[0]["cmcontinue"]) + self.assertEqual(self.sent_params[1]["cmcontinue"], "page|5350454c4c2033|123") + self.assertEqual(self.sent_params[1]["cmtitle"], "Category:Spells") + + def test_articles_are_requested_in_batches_of_fifty(self): + first_batch = [f"Article {n}" for n in range(50)] + second_batch = [f"Article {n}" for n in range(50, 60)] + self.respond(response(200, revisions_body(first_batch)), response(200, revisions_body(second_batch))) + + titles = [article.title for article in self.wiki_client.get_articles(first_batch + second_batch)] + + self.assertEqual(titles, first_batch + second_batch) + self.assert_requests(2) + self.assertEqual( + [params["titles"] for params in self.sent_params], + ["|".join(first_batch), "|".join(second_batch)], + ) + self.sleep.assert_not_called() + + def test_body_cut_short_is_retried(self): + self.respond( + requests.exceptions.ChunkedEncodingError("connection broken mid-body"), + response(200, load_resource("response_revisions.json")), + ) + + article = self.wiki_client.get_article("Golden Armor") + + self.assertEqual(article.title, "Golden Armor") + self.assert_requests(2) + self.assertEqual(self.sleep.call_args_list, [call(5)]) + + def test_bodies_cut_short_raise_after_five_attempts(self): + self.respond(*[requests.exceptions.ChunkedEncodingError("connection broken mid-body")] * 5) + + with self.assertRaises(WikiApiError) as context: + self.wiki_client.get_article("Golden Armor") + + error = context.exception + self.assertIsNone(error.status) + self.assertIsNone(error.body) + self.assertIsInstance(error.__cause__, requests.exceptions.ChunkedEncodingError) + self.assertIn("Golden Armor", str(error)) + self.assert_requests(5) + self.assertEqual(self.sleep.call_args_list, [call(5), call(15), call(30), call(60)]) + + def test_body_that_is_never_json_raises_after_five_attempts(self): + body = "" + "x" * 300 + self.respond(*[response(200, body)] * 5) + + with self.assertRaises(WikiApiError) as context: + self.wiki_client.get_article("Golden Armor") + + error = context.exception + self.assertEqual(error.status, 200) + self.assertEqual(error.body, body[:200]) + self.assertIsInstance(error.__cause__, ValueError) + self.assertIn("status 200", str(error)) + self.assertIn("Golden Armor", str(error)) + self.assert_requests(5) + self.assertEqual(self.sleep.call_args_list, [call(5), call(15), call(30), call(60)]) + + def test_timeouts_raise_after_five_attempts(self): + self.respond(*[requests.Timeout("read timed out")] * 5) + + with self.assertRaises(WikiApiError) as context: + self.wiki_client.get_article("Golden Armor") + + error = context.exception + self.assertIsNone(error.status) + self.assertIsNone(error.body) + self.assertIsInstance(error.__cause__, requests.Timeout) + self.assertIn("no response", str(error)) + self.assertIn("Golden Armor", str(error)) + self.assert_requests(5) + self.assertEqual(self.sleep.call_args_list, [call(5), call(15), call(30), call(60)]) + + def test_not_found_raises_at_once(self): + self.respond(response(404, "Not Found")) + + with self.assertRaises(WikiApiError) as context: + self.wiki_client.get_article("Golden Armor") + + error = context.exception + self.assertEqual(error.status, 404) + self.assertEqual(error.body, "Not Found") + self.assertIn("status 404", str(error)) + self.assertIn("Golden Armor", str(error)) + self.assert_requests(1) + self.sleep.assert_not_called() + + def test_api_error_raises_at_once(self): + body = json.dumps({"error": {"code": "badtitle", "info": "Bad title \"\"."}}) + self.respond(response(200, body)) + + with self.assertRaises(WikiApiError) as context: + self.wiki_client.get_article("") + + error = context.exception + self.assertEqual(error.status, 200) + self.assertEqual(error.body, body) + self.assertIn("badtitle", str(error)) + self.assert_requests(1) + self.sleep.assert_not_called() + + def test_articles_of_a_retried_batch_are_yielded_once(self): + first_batch = [f"Article {n}" for n in range(50)] + second_batch = [f"Article {n}" for n in range(50, 60)] + self.respond( + response(200, revisions_body(first_batch)), + response(503, "Service Unavailable"), + response(200, revisions_body(second_batch)), + ) + + titles = [article.title for article in self.wiki_client.get_articles(first_batch + second_batch)] + + self.assertEqual(titles, first_batch + second_batch) + self.assert_requests(3) + self.assertEqual( + [params["titles"] for params in self.sent_params], + ["|".join(first_batch), "|".join(second_batch), "|".join(second_batch)], + ) + self.assertEqual(self.sleep.call_args_list, [call(5)]) + + def test_images_info_stops_after_not_found(self): + self.respond(response(404, "Not Found")) + + with self.assertRaises(WikiApiError): + list(self.wiki_client.get_images_info(["Golden Armor.gif"])) + + self.assert_requests(1) + self.sleep.assert_not_called() + + def test_images_info_stops_after_retry_budget(self): + self.respond(*[response(502, "Bad Gateway")] * 5) + + with self.assertRaises(WikiApiError) as context: + list(self.wiki_client.get_images_info(["Golden Armor.gif"])) + + self.assertEqual(context.exception.status, 502) + self.assert_requests(5) + self.assertEqual(self.sleep.call_args_list, [call(5), call(15), call(30), call(60)]) diff --git a/tibiawikisql/api.py b/tibiawikisql/api.py index 6edd990..d51fda7 100644 --- a/tibiawikisql/api.py +++ b/tibiawikisql/api.py @@ -1,18 +1,25 @@ +# Changed by tibia.sh in 2026. See "About this copy" in README.md. """API to fetch information from [TibiaWiki](https://tibia.fandom.com) through MediaWiki's API.""" import datetime import json +import time import urllib.parse -from collections.abc import Generator +from collections.abc import Callable, Generator from typing import ClassVar from pydantic import BaseModel, computed_field import requests from tibiawikisql import __version__ +from tibiawikisql.errors import WikiApiError from tibiawikisql.utils import parse_templatates_data BASE_URL = "https://tibia.fandom.com" +REQUEST_TIMEOUT = 60 +"""Seconds to wait for each response of the API.""" +RETRY_DELAYS = (5, 15, 30, 60) +"""Seconds to wait before each retry of a failed API request, so a request is attempted up to 5 times.""" class WikiEntry(BaseModel): @@ -89,9 +96,68 @@ class WikiClient: "User-Agent": f'tibiawikisql/{__version__}', # noqa: Q000 } - def __init__(self) -> None: - """Creates a new instance of the client.""" + def __init__(self, sleep: Callable[[float], None] = time.sleep) -> None: + """Creates a new instance of the client. + + Args: + sleep: The function used to wait between attempts of a request. + + """ self.session = requests.Session() + self._sleep = sleep + + def _get_json(self, params: dict) -> dict: + """Request the API and parse its response. + + A request that gets no response, a response cut short mid-body, a 429 or 5xx status, or a 200 status with a + body that is not JSON is retried after each of the `RETRY_DELAYS`. + + Args: + params: The query parameters of the request. + + Returns: + The parsed response. + + Raises: + WikiApiError: The response had another status or held an API error, or the last attempt failed. + + """ + attempt = 0 + while True: + attempt += 1 + cause = None + try: + response = self.session.get(self.ENDPOINT, params=params, timeout=REQUEST_TIMEOUT) + except (requests.ConnectionError, requests.Timeout, requests.exceptions.ChunkedEncodingError) as exc: + cause = exc + status = text = None + problem = f"no response ({exc.__class__.__name__}: {exc})" + else: + status = response.status_code + text = response.text + if status == 200: + try: + data = json.loads(text) + except ValueError as exc: + cause = exc + problem = "status 200 with a body that is not JSON" + else: + if "error" in data: + error = data["error"] + code = error.get("code") if isinstance(error, dict) else error + msg = f"Wiki API error {code!r} with status 200 for {self.ENDPOINT} with params {params}" + raise WikiApiError(msg, status, text) + return data + elif status == 429 or 500 <= status < 600: + problem = f"status {status}" + else: + msg = f"Wiki API request failed with status {status} for {self.ENDPOINT} with params {params}" + raise WikiApiError(msg, status, text) + if attempt > len(RETRY_DELAYS): + msg = (f"Wiki API request failed with {problem} for {self.ENDPOINT} with params {params}, " + f"after {attempt} attempts") + raise WikiApiError(msg, status, text) from cause + self._sleep(RETRY_DELAYS[attempt - 1]) def get_category_members(self, name: str, skip_index: bool = True) -> Generator[WikiEntry]: """Create a generator that obtains entries in a certain category. @@ -116,8 +182,7 @@ def get_category_members(self, name: str, skip_index: bool = True) -> Generator[ } while True: params["cmcontinue"] = cmcontinue - r = self.session.get(self.ENDPOINT, params=params) - data = json.loads(r.text) + data = self._get_json(params) for member in data["query"]["categorymembers"]: if member["sortkeyprefix"] == "*" and skip_index: continue @@ -189,10 +254,7 @@ def get_images_info(self, names: list[str]) -> Generator[Image | None]: break params["titles"] = "|".join(f"File:{n}" for n in names[i:min(i + 50, len(names))]) - r = self.session.get(self.ENDPOINT, params=params) - if r.status_code >= 400: - continue - data = json.loads(r.text) + data = self._get_json(params) i += 50 for image_data in data["query"]["pages"].values(): if "missing" in image_data: @@ -233,8 +295,7 @@ def get_articles(self, names: list[str]) -> Generator[Article | None]: break params["titles"] = "|".join(names[i:min(i + 50, len(names))]) i += 50 - r = self.session.get(self.ENDPOINT, params=params) - data = json.loads(r.text) + data = self._get_json(params) for article in data["query"]["pages"].values(): if "missing" in article: yield None diff --git a/tibiawikisql/errors.py b/tibiawikisql/errors.py index a318903..d0ad971 100644 --- a/tibiawikisql/errors.py +++ b/tibiawikisql/errors.py @@ -1,3 +1,4 @@ +# Changed by tibia.sh in 2026. See "About this copy" in README.md. """Custom exceptions used by the package.""" from __future__ import annotations @@ -57,6 +58,25 @@ def __init__(self, article: Article, parser: type[BaseParser]) -> None: super().__init__(article, f"Template `{parser.template_name}` not found.") +class WikiApiError(TibiaWikiSqlError): + """Error raised when a request to TibiaWiki's API fails. + + The exception that caused it, if any, is kept as ``__cause__``. + """ + + def __init__(self, message: str, status: int | None, body: str | None) -> None: + """Create an instance of the class. + + Args: + message: A description of the failed request. + status: The response's HTTP status, or `None` if there was no response. + body: The response's text, or `None` if there was no response. Only its first 200 characters are kept. + """ + super().__init__(message) + self.status = status + self.body = None if body is None else body[:200] + + class DatabaseError(TibiaWikiSqlError): """Error raised when a database related error happens."""