From e7440a2dfcb62931ba00a0f250cf0aa926e50307 Mon Sep 17 00:00:00 2001 From: Sam Dornan Date: Tue, 18 Aug 2026 20:19:26 -0500 Subject: [PATCH 1/3] fix(hltb): fetch metadata when a HowLongToBeat ID is set by hand `update_rom` fetches from every metadata provider whose ID changed, except HLTB, which had no by-ID lookup at all. A hand-entered ID was stored and the How Long to Beat tab stayed empty. HLTB's API only exposes search, so `get_rom_by_id` reads the record from the `__NEXT_DATA__` payload the game page already ships. The canonical `/game/{id}` path is requested directly, as `/game?id=` answers with a redirect the shared httpx client does not follow, and the game page dates a release in full where search returns just the year. A page RomM can no longer read raises 502 naming the cause instead of reporting a game with no times, since that failure is otherwise indistinguishable from the bug this fixes. Fixes #2926 Co-Authored-By: Claude Opus 5 --- backend/endpoints/roms/__init__.py | 8 + backend/handler/metadata/hltb_handler.py | 215 +++++++++++++---- backend/tests/endpoints/roms/test_rom.py | 42 +++- .../metadata/hltb_game_page_example.json | 32 +++ .../handler/metadata/test_hltb_handler.py | 216 ++++++++++++++++++ 5 files changed, 461 insertions(+), 52 deletions(-) create mode 100644 backend/tests/handler/metadata/hltb_game_page_example.json diff --git a/backend/endpoints/roms/__init__.py b/backend/endpoints/roms/__init__.py index e1375e5d5..107197925 100644 --- a/backend/endpoints/roms/__init__.py +++ b/backend/endpoints/roms/__init__.py @@ -59,6 +59,7 @@ from handler.filesystem.assets_handler import validate_image_upload from handler.metadata import ( meta_flashpoint_handler, + meta_hltb_handler, meta_igdb_handler, meta_launchbox_handler, meta_moby_handler, @@ -1790,6 +1791,13 @@ async def update_rom( elif rom.igdb_id and not cleaned_data["igdb_id"]: cleaned_data.update({"igdb_id": None, "igdb_metadata": {}}) + if cleaned_data["hltb_id"] and int(cleaned_data["hltb_id"]) != rom.hltb_id: + hltb_rom = await meta_hltb_handler.get_rom_by_id(int(cleaned_data["hltb_id"])) + if hltb_rom.get("hltb_id"): + cleaned_data.update(hltb_rom) + elif rom.hltb_id and not cleaned_data["hltb_id"]: + cleaned_data.update({"hltb_id": None, "hltb_metadata": {}}) + url_screenshots = cleaned_data.get("url_screenshots", []) screenshots_changed = pydash.xor(url_screenshots, rom.url_screenshots or []) if url_screenshots: diff --git a/backend/handler/metadata/hltb_handler.py b/backend/handler/metadata/hltb_handler.py index 7d0e2a89b..3465bb497 100644 --- a/backend/handler/metadata/hltb_handler.py +++ b/backend/handler/metadata/hltb_handler.py @@ -5,6 +5,7 @@ from typing import Final, NotRequired, TypedDict import httpx +import pydash from fastapi import HTTPException, status from config import HLTB_API_ENABLED @@ -19,6 +20,10 @@ # Regex to detect HLTB ID tags in filenames like (hltb-12345) HLTB_TAG_REGEX = re.compile(r"\(hltb-(\d+)\)", re.IGNORECASE) DASH_COLON_REGEX = re.compile(r"\s?-\s") +# The game page ships its record as JSON in the Next.js hydration payload. +NEXT_DATA_REGEX = re.compile( + r'', re.DOTALL +) # HLTB publishes no rate limit, so stay well clear of being throttled. HLTB_MAX_REQUESTS_PER_SECOND: Final[float] = 3 @@ -131,6 +136,60 @@ class HLTBRom(BaseRom): hltb_metadata: NotRequired[HLTBMetadata] +def _release_year(value: object) -> int: + """Normalize a release date: search returns a year, the game page an ISO date.""" + if isinstance(value, int): + return value + if isinstance(value, str): + year, _, _ = value.partition("-") + if year.isdigit(): + return int(year) + return 0 + + +def build_hltb_game(game_data: dict) -> HLTBGame: + """Build an HLTBGame, defaulting the fields a given HLTB payload omits.""" + return HLTBGame( + game_id=game_data.get("game_id", 0), + game_name=game_data.get("game_name", ""), + game_name_date=game_data.get("game_name_date", 0), + game_alias=game_data.get("game_alias", ""), + game_type=game_data.get("game_type", ""), + game_image=game_data.get("game_image", ""), + comp_lvl_combine=game_data.get("comp_lvl_combine", 0), + comp_lvl_sp=game_data.get("comp_lvl_sp", 0), + comp_lvl_co=game_data.get("comp_lvl_co", 0), + comp_lvl_mp=game_data.get("comp_lvl_mp", 0), + comp_main=game_data.get("comp_main", 0), + comp_plus=game_data.get("comp_plus", 0), + comp_100=game_data.get("comp_100", 0), + comp_all=game_data.get("comp_all", 0), + comp_main_count=game_data.get("comp_main_count", 0), + comp_plus_count=game_data.get("comp_plus_count", 0), + comp_100_count=game_data.get("comp_100_count", 0), + comp_all_count=game_data.get("comp_all_count", 0), + invested_co=game_data.get("invested_co", 0), + invested_mp=game_data.get("invested_mp", 0), + invested_co_count=game_data.get("invested_co_count", 0), + invested_mp_count=game_data.get("invested_mp_count", 0), + count_comp=game_data.get("count_comp", 0), + count_speedrun=game_data.get("count_speedrun", 0), + count_backlog=game_data.get("count_backlog", 0), + count_review=game_data.get("count_review", 0), + review_score=game_data.get("review_score", 0), + count_playing=game_data.get("count_playing", 0), + count_retired=game_data.get("count_retired", 0), + profile_platform=game_data.get("profile_platform", ""), + profile_popular=game_data.get("profile_popular", 0) or 0, + release_world=_release_year(game_data.get("release_world")), + ) + + +def build_cover_url(game: HLTBGame) -> str: + image = game.get("game_image") + return f"https://howlongtobeat.com/games/{image}" if image else "" + + def extract_hltb_metadata(game: HLTBGame) -> HLTBMetadata: """Extract metadata from HLTB game data.""" metadata = HLTBMetadata() @@ -184,6 +243,21 @@ def extract_hltb_metadata(game: HLTBGame) -> HLTBMetadata: GITHUB_FILE_URL = "https://raw.githubusercontent.com/rommapp/romm/refs/heads/master/backend/handler/metadata/fixtures/hltb_api_url" +# Raised where the page parsed but did not carry the shape we read, so a +# rewrite upstream surfaces as itself instead of as a game with no times. +HLTB_FORMAT_CHANGED_DETAIL: Final[str] = ( + "HowLongToBeat changed their game page, RomM could not read the game data" +) + + +def _format_changed(reason: str) -> HTTPException: + log.error("HowLongToBeat game page could not be read: %s", reason) + return HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=HLTB_FORMAT_CHANGED_DETAIL, + ) + + def _unavailable_detail(status_code: int) -> str: """Describe why HLTB is unusable, so the cause isn't misreported as a network fault.""" if status_code == status.HTTP_403_FORBIDDEN: @@ -455,42 +529,7 @@ async def search_games( games = [] for game_data in games_data: if isinstance(game_data, dict) and "game_id" in game_data: - # Create HLTBGame with all required fields, using defaults for missing ones - hltb_game = HLTBGame( - game_id=game_data.get("game_id", 0), - game_name=game_data.get("game_name", ""), - game_name_date=game_data.get("game_name_date", 0), - game_alias=game_data.get("game_alias", ""), - game_type=game_data.get("game_type", ""), - game_image=game_data.get("game_image", ""), - comp_lvl_combine=game_data.get("comp_lvl_combine", 0), - comp_lvl_sp=game_data.get("comp_lvl_sp", 0), - comp_lvl_co=game_data.get("comp_lvl_co", 0), - comp_lvl_mp=game_data.get("comp_lvl_mp", 0), - comp_main=game_data.get("comp_main", 0), - comp_plus=game_data.get("comp_plus", 0), - comp_100=game_data.get("comp_100", 0), - comp_all=game_data.get("comp_all", 0), - comp_main_count=game_data.get("comp_main_count", 0), - comp_plus_count=game_data.get("comp_plus_count", 0), - comp_100_count=game_data.get("comp_100_count", 0), - comp_all_count=game_data.get("comp_all_count", 0), - invested_co=game_data.get("invested_co", 0), - invested_mp=game_data.get("invested_mp", 0), - invested_co_count=game_data.get("invested_co_count", 0), - invested_mp_count=game_data.get("invested_mp_count", 0), - count_comp=game_data.get("count_comp", 0), - count_speedrun=game_data.get("count_speedrun", 0), - count_backlog=game_data.get("count_backlog", 0), - count_review=game_data.get("count_review", 0), - review_score=game_data.get("review_score", 0), - count_playing=game_data.get("count_playing", 0), - count_retired=game_data.get("count_retired", 0), - profile_platform=game_data.get("profile_platform", ""), - profile_popular=game_data.get("profile_popular", 0), - release_world=game_data.get("release_world", 0), - ) - games.append(hltb_game) + games.append(build_hltb_game(game_data)) return games except Exception as exc: @@ -593,17 +632,10 @@ async def _search_and_score( f"Found HowLongToBeat match for '{search_term}' -> '{best_match}' (score: {best_score:.3f})" ) - # Build cover URL if image is available - cover_url = "" - if best_game.get("game_image"): - cover_url = ( - f"https://howlongtobeat.com/games/{best_game['game_image']}" - ) - return HLTBRom( hltb_id=best_game["game_id"], name=best_game["game_name"], - url_cover=cover_url, + url_cover=build_cover_url(best_game), hltb_metadata=extract_hltb_metadata(best_game), ) @@ -628,22 +660,105 @@ async def get_matched_roms_by_name( roms = [] for game in games: - # Build cover URL if image is available - cover_url = "" - if game.get("game_image"): - cover_url = f"https://howlongtobeat.com/games/{game['game_image']}" - roms.append( HLTBRom( hltb_id=game["game_id"], name=game["game_name"], - url_cover=cover_url, + url_cover=build_cover_url(game), hltb_metadata=extract_hltb_metadata(game), ) ) return roms + async def get_rom_by_id(self, hltb_id: int) -> HLTBRom: + """ + Get ROM information from HowLongToBeat by its game ID. + + HLTB's API only exposes search, so the record is read from the hydration + payload the game page already ships to the browser. + + :param hltb_id: The HowLongToBeat game ID. + :return: A HLTBRom object. + """ + if not self.is_enabled(): + return HLTBRom(hltb_id=None) + + game_data = await self._fetch_game_page(hltb_id) + if not game_data: + return HLTBRom(hltb_id=None) + + game = build_hltb_game(game_data) + + return HLTBRom( + hltb_id=game["game_id"], + name=game["game_name"], + url_cover=build_cover_url(game), + hltb_metadata=extract_hltb_metadata(game), + ) + + async def _fetch_game_page(self, hltb_id: int) -> dict: + """Fetch and parse a game page's hydration payload.""" + httpx_client = ctx_httpx_client.get() + + # The page is HLTB traffic like any other, so it respects the same cap. + await _rate_limiter.acquire() + + try: + # Request the canonical path directly: the `/game?id=` form answers + # with a redirect, which the shared client does not follow. + res = await httpx_client.get( + f"{self.base_url}/game/{hltb_id}", + headers=self._base_headers(), + timeout=60, + ) + res.raise_for_status() + except httpx.HTTPStatusError as exc: + status_code = exc.response.status_code + if status_code == status.HTTP_404_NOT_FOUND: + log.debug("HowLongToBeat has no game with ID %s", hltb_id) + return {} + + log.warning( + "HowLongToBeat game page returned HTTP %s", status_code, exc_info=True + ) + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=_unavailable_detail(status_code), + ) from exc + except (httpx.ConnectError, httpx.ReadTimeout) as exc: + log.warning( + "Connection error: can't connect to HowLongToBeat", exc_info=True + ) + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Can't connect to HowLongToBeat API, check your internet connection", + ) from exc + + match = NEXT_DATA_REGEX.search(res.text) + if not match: + raise _format_changed("the page carried no hydration payload") + + try: + payload = json.loads(match.group(1)) + except json.JSONDecodeError as exc: + raise _format_changed(f"the hydration payload is not JSON: {exc}") from exc + + games = pydash.get(payload, "props.pageProps.game.data.game") + if not isinstance(games, list): + raise _format_changed("the hydration payload holds no game records") + + # An empty list is HLTB answering honestly, not a rewrite: the ID is gone. + if not games: + log.debug("HowLongToBeat has no record for game ID %s", hltb_id) + return {} + + game_data = games[0] + if not isinstance(game_data, dict) or "game_id" not in game_data: + raise _format_changed("the game record is not in the expected shape") + + return game_data + async def price_check( self, hltb_id: int, steam_id: int = 0, itch_id: int = 0 ) -> HLTBPriceCheckResponse | None: diff --git a/backend/tests/endpoints/roms/test_rom.py b/backend/tests/endpoints/roms/test_rom.py index ff191ef73..f982df60e 100644 --- a/backend/tests/endpoints/roms/test_rom.py +++ b/backend/tests/endpoints/roms/test_rom.py @@ -10,6 +10,7 @@ from handler.filesystem.resources_handler import FSResourcesHandler from handler.filesystem.roms_handler import FSRomsHandler from handler.metadata.flashpoint_handler import FlashpointHandler, FlashpointRom +from handler.metadata.hltb_handler import HLTBHandler, HLTBRom from handler.metadata.igdb_handler import IGDBHandler, IGDBRom from handler.metadata.launchbox_handler.handler import LaunchboxHandler from handler.metadata.launchbox_handler.types import LaunchboxRom @@ -1406,8 +1407,40 @@ def test_update_rom_hasheous_id( body = response.json() assert body["hasheous_id"] == MOCK_HASHEOUS_ID - def test_update_rom_hltb_id(self, client: TestClient, access_token: str, rom: Rom): - """Test updating HowLongToBeat ID.""" + @patch.object( + HLTBHandler, + "get_rom_by_id", + return_value=HLTBRom(hltb_id=MOCK_HLTB_ID, hltb_metadata={"main_story": 92822}), + ) + def test_update_rom_hltb_id( + self, + get_rom_by_id_mock: AsyncMock, + client: TestClient, + access_token: str, + rom: Rom, + ): + """A hand-entered HowLongToBeat ID has to pull its times down with it.""" + response = client.put( + f"/api/roms/{rom.id}", + headers={"Authorization": f"Bearer {access_token}"}, + data={"hltb_id": str(MOCK_HLTB_ID)}, + ) + assert response.status_code == status.HTTP_200_OK + + body = response.json() + assert body["hltb_id"] == MOCK_HLTB_ID + assert body["hltb_metadata"]["main_story"] == 92822 + assert get_rom_by_id_mock.called + + @patch.object(HLTBHandler, "get_rom_by_id", return_value=HLTBRom(hltb_id=None)) + def test_update_rom_hltb_id_persists_when_handler_disabled( + self, + get_rom_by_id_mock: AsyncMock, + client: TestClient, + access_token: str, + rom: Rom, + ): + """Test that HLTB ID persists when handler is disabled or game not found.""" response = client.put( f"/api/roms/{rom.id}", headers={"Authorization": f"Bearer {access_token}"}, @@ -1417,6 +1450,7 @@ def test_update_rom_hltb_id(self, client: TestClient, access_token: str, rom: Ro body = response.json() assert body["hltb_id"] == MOCK_HLTB_ID + assert get_rom_by_id_mock.called class TestUpdateRawMetadata: @@ -1617,8 +1651,12 @@ def test_update_raw_flashpoint_metadata( assert body["flashpoint_metadata"]["companies"] == ["Nintendo"] assert body["flashpoint_metadata"]["source"] == "Flashpoint" + @patch.object( + HLTBHandler, "get_rom_by_id", return_value=HLTBRom(hltb_id=MOCK_HLTB_ID) + ) def test_update_raw_hltb_metadata( self, + get_rom_by_id_mock: AsyncMock, client: TestClient, access_token: str, rom: Rom, diff --git a/backend/tests/handler/metadata/hltb_game_page_example.json b/backend/tests/handler/metadata/hltb_game_page_example.json new file mode 100644 index 000000000..c30bae0a7 --- /dev/null +++ b/backend/tests/handler/metadata/hltb_game_page_example.json @@ -0,0 +1,32 @@ +{ + "props": { + "pageProps": { + "game": { + "data": { + "game": [ + { + "game_id": 7169, + "game_name": "Pokémon Red and Blue", + "count_comp": 5364, + "count_review": 1762, + "review_score": 81, + "game_alias": "Pokemon Red and Blue, Pokemon Green, Pocket Monsters Red, Pocket Monsters Green, Pocket Monsters Blue, Pokemon Blue Version, Pokemon Red Version, Pokemon Green Version", + "game_image": "7169_Pokmon_Red_and_Blue.png", + "game_type": "game", + "profile_platform": "Game Boy", + "release_world": "1996-02-27", + "comp_all_count": 1174, + "comp_all": 139926, + "comp_main_count": 594, + "comp_main": 92822, + "comp_plus_count": 378, + "comp_plus": 156016, + "comp_100_count": 202, + "comp_100": 354756 + } + ] + } + } + } + } +} diff --git a/backend/tests/handler/metadata/test_hltb_handler.py b/backend/tests/handler/metadata/test_hltb_handler.py index fa2212b11..658499cc9 100644 --- a/backend/tests/handler/metadata/test_hltb_handler.py +++ b/backend/tests/handler/metadata/test_hltb_handler.py @@ -1,3 +1,5 @@ +import json +from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import httpx @@ -435,3 +437,217 @@ async def search_games(_term, _platform_slug): rom = await handler.get_rom("007 - Quantum of Solace (USA).chd", "ps2") assert rom["hltb_id"] is None + + +def _game_page(game: dict | None) -> MagicMock: + """A game page carrying its record in the Next.js hydration payload.""" + games = [game] if game is not None else [] + payload = json.dumps({"props": {"pageProps": {"game": {"data": {"game": games}}}}}) + response = MagicMock() + response.status_code = 200 + response.text = ( + '" + ) + return response + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_get_rom_by_id_reads_the_game_page(mock_ctx_httpx_client): + """The by-ID lookup the manual edit form depends on: HLTB has no API for it, + so the record comes off the game page.""" + handler = _handler() + client = MagicMock() + client.get = AsyncMock(return_value=_game_page(_game(7169, "Pokémon Red and Blue"))) + mock_ctx_httpx_client.get.return_value = client + + rom = await handler.get_rom_by_id(7169) + + assert rom["hltb_id"] == 7169 + assert rom["name"] == "Pokémon Red and Blue" + assert rom["hltb_metadata"]["main_story"] == 3600 + assert rom["hltb_metadata"]["review_score"] == 70 + + # The `/game?id=` form answers with a redirect the shared client won't follow. + assert client.get.await_args.args[0] == "https://howlongtobeat.com/game/7169" + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_get_rom_by_id_sends_the_user_agent_hltb_requires( + mock_ctx_httpx_client, +): + handler = _handler() + client = MagicMock() + client.get = AsyncMock(return_value=_game_page(_game(7169, "Pokémon"))) + mock_ctx_httpx_client.get.return_value = client + + await handler.get_rom_by_id(7169) + + assert ( + client.get.await_args.kwargs["headers"]["User-Agent"] == f"RomM/{get_version()}" + ) + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_get_rom_by_id_reads_the_full_release_date(mock_ctx_httpx_client): + """The game page dates a release in full where search returns just the year.""" + handler = _handler() + game = _game(7169, "Pokémon Red and Blue") | {"release_world": "1996-02-27"} + client = MagicMock() + client.get = AsyncMock(return_value=_game_page(game)) + mock_ctx_httpx_client.get.return_value = client + + rom = await handler.get_rom_by_id(7169) + + assert rom["hltb_metadata"]["release_year"] == 1996 + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_get_rom_by_id_tolerates_a_page_without_popularity( + mock_ctx_httpx_client, +): + """The game page omits the popularity search reports, so it must not fail.""" + handler = _handler() + game = _game(7169, "Pokémon Red and Blue") | {"profile_popular": None} + client = MagicMock() + client.get = AsyncMock(return_value=_game_page(game)) + mock_ctx_httpx_client.get.return_value = client + + rom = await handler.get_rom_by_id(7169) + + assert rom["hltb_id"] == 7169 + assert "popularity" not in rom["hltb_metadata"] + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_unknown_id_is_not_reported_as_an_outage(mock_ctx_httpx_client): + """A mistyped ID is the user's, not HLTB's, so it must not 503 the edit.""" + handler = _handler() + client = MagicMock() + client.get = AsyncMock(return_value=_response(status.HTTP_404_NOT_FOUND)) + mock_ctx_httpx_client.get.return_value = client + + rom = await handler.get_rom_by_id(999999999) + + assert rom["hltb_id"] is None + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_game_page_outage_reports_service_unavailable(mock_ctx_httpx_client): + handler = _handler() + client = MagicMock() + client.get = AsyncMock( + return_value=_response(status.HTTP_429_TOO_MANY_REQUESTS), + ) + mock_ctx_httpx_client.get.return_value = client + + with pytest.raises(HTTPException) as exc_info: + await handler.get_rom_by_id(7169) + + assert exc_info.value.status_code == status.HTTP_503_SERVICE_UNAVAILABLE + assert "rate limiting" in exc_info.value.detail + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_game_page_without_a_record_yields_no_match(mock_ctx_httpx_client): + """An empty record list is HLTB answering honestly, not a rewrite.""" + handler = _handler() + client = MagicMock() + client.get = AsyncMock(return_value=_game_page(None)) + mock_ctx_httpx_client.get.return_value = client + + rom = await handler.get_rom_by_id(7169) + + assert rom["hltb_id"] is None + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_reshaped_game_page_reports_the_rewrite(mock_ctx_httpx_client): + """A page RomM can no longer read has to say so: reported as a game with no + times it is indistinguishable from the bug this lookup exists to fix.""" + handler = _handler() + response = MagicMock() + response.status_code = 200 + response.text = "no hydration payload here" + client = MagicMock() + client.get = AsyncMock(return_value=response) + mock_ctx_httpx_client.get.return_value = client + + with pytest.raises(HTTPException) as exc_info: + await handler.get_rom_by_id(7169) + + assert exc_info.value.status_code == status.HTTP_502_BAD_GATEWAY + assert "changed their game page" in exc_info.value.detail + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_renamed_game_fields_report_the_rewrite(mock_ctx_httpx_client): + """Defaulting every field would quietly turn a rename into an empty match.""" + handler = _handler() + client = MagicMock() + client.get = AsyncMock(return_value=_game_page({"id": 7169, "name": "Pokémon"})) + mock_ctx_httpx_client.get.return_value = client + + with pytest.raises(HTTPException) as exc_info: + await handler.get_rom_by_id(7169) + + assert exc_info.value.status_code == status.HTTP_502_BAD_GATEWAY + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", False) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_get_rom_by_id_is_skipped_when_hltb_is_disabled(mock_ctx_httpx_client): + rom = await _handler().get_rom_by_id(7169) + + assert rom["hltb_id"] is None + mock_ctx_httpx_client.get.assert_not_called() + + +async def test_the_live_page_shape_still_parses(): + """Captured from howlongtobeat.com/game/7169. If HLTB reshapes the page this + fixture goes stale, but it keeps our own parsing honest in the meantime.""" + fixture = Path(__file__).parent / "hltb_game_page_example.json" + payload = json.loads(fixture.read_text(encoding="utf-8")) + + response = MagicMock() + response.status_code = 200 + response.text = ( + '" + ) + client = MagicMock() + client.get = AsyncMock(return_value=response) + + with ( + patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True), + patch("handler.metadata.hltb_handler.ctx_httpx_client") as ctx, + ): + ctx.get.return_value = client + rom = await _handler().get_rom_by_id(7169) + + assert rom["hltb_id"] == 7169 + assert rom["name"] == "Pokémon Red and Blue" + assert rom["url_cover"].endswith("7169_Pokmon_Red_and_Blue.png") + assert rom["hltb_metadata"] == { + "main_story": 92822, + "main_story_count": 594, + "main_plus_extra": 156016, + "main_plus_extra_count": 378, + "completionist": 354756, + "completionist_count": 202, + "all_styles": 139926, + "all_styles_count": 1174, + "release_year": 1996, + "review_score": 81, + "review_count": 1762, + "completions": 5364, + } From 03fe9921013a91a11260c5ef0dfa01f6904ab480 Mon Sep 17 00:00:00 2001 From: Sam Dornan Date: Tue, 18 Aug 2026 20:30:28 -0500 Subject: [PATCH 2/3] fix(hltb): map every transport failure on the game page to 503 `ConnectTimeout` is the likely failure when HLTB is slow or unreachable, and it is not a subclass of `ConnectError`, so it escaped `update_rom` as a bare 500 rather than the actionable 503 the other providers give. Co-Authored-By: Claude Opus 5 --- backend/handler/metadata/hltb_handler.py | 4 ++- .../handler/metadata/test_hltb_handler.py | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/backend/handler/metadata/hltb_handler.py b/backend/handler/metadata/hltb_handler.py index 3465bb497..fa4f08567 100644 --- a/backend/handler/metadata/hltb_handler.py +++ b/backend/handler/metadata/hltb_handler.py @@ -726,7 +726,9 @@ async def _fetch_game_page(self, hltb_id: int) -> dict: status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=_unavailable_detail(status_code), ) from exc - except (httpx.ConnectError, httpx.ReadTimeout) as exc: + # Broader than the search path's catch: a connect timeout is the likely + # failure here, and it would otherwise escape update_rom as a bare 500. + except httpx.RequestError as exc: log.warning( "Connection error: can't connect to HowLongToBeat", exc_info=True ) diff --git a/backend/tests/handler/metadata/test_hltb_handler.py b/backend/tests/handler/metadata/test_hltb_handler.py index 658499cc9..8462468c6 100644 --- a/backend/tests/handler/metadata/test_hltb_handler.py +++ b/backend/tests/handler/metadata/test_hltb_handler.py @@ -651,3 +651,30 @@ async def test_the_live_page_shape_still_parses(): "review_count": 1762, "completions": 5364, } + + +@pytest.mark.parametrize( + "transport_error", + [ + httpx.ConnectTimeout("timed out"), + httpx.PoolTimeout("pool exhausted"), + httpx.ReadError("reset"), + httpx.RemoteProtocolError("bad framing"), + ], +) +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_transport_failures_report_service_unavailable( + mock_ctx_httpx_client, transport_error +): + """A slow or unreachable HLTB must not surface as a bare 500 on the rom edit.""" + handler = _handler() + client = MagicMock() + client.get = AsyncMock(side_effect=transport_error) + mock_ctx_httpx_client.get.return_value = client + + with pytest.raises(HTTPException) as exc_info: + await handler.get_rom_by_id(7169) + + assert exc_info.value.status_code == status.HTTP_503_SERVICE_UNAVAILABLE + assert "check your internet connection" in exc_info.value.detail From 808745e7ae61f335daf42cdb75581f36f1aeb283 Mon Sep 17 00:00:00 2001 From: Sam Dornan Date: Tue, 18 Aug 2026 20:38:33 -0500 Subject: [PATCH 3/3] fix(hltb): don't mistake markup or a redirect for a rewritten page Two ways the 502 could fire on a page RomM can still read: The hydration tag was matched by exact attribute order, so a CSP nonce or a reordered attribute would have been reported as a rewrite. The id alone identifies it. `raise_for_status` lets a 3xx through, so a hop HLTB added later would reach the parser as a page with no payload. Redirects are now followed; the client validates every hop against SSRF, redirects included. Co-Authored-By: Claude Opus 5 --- backend/handler/metadata/hltb_handler.py | 13 ++-- .../handler/metadata/test_hltb_handler.py | 71 +++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/backend/handler/metadata/hltb_handler.py b/backend/handler/metadata/hltb_handler.py index fa4f08567..c550c3337 100644 --- a/backend/handler/metadata/hltb_handler.py +++ b/backend/handler/metadata/hltb_handler.py @@ -20,9 +20,11 @@ # Regex to detect HLTB ID tags in filenames like (hltb-12345) HLTB_TAG_REGEX = re.compile(r"\(hltb-(\d+)\)", re.IGNORECASE) DASH_COLON_REGEX = re.compile(r"\s?-\s") -# The game page ships its record as JSON in the Next.js hydration payload. +# The game page ships its record as JSON in the Next.js hydration payload. The +# id alone identifies the tag, so attribute order and extras a CSP would add +# (nonce, crossorigin) do not read as a rewritten page. NEXT_DATA_REGEX = re.compile( - r'', re.DOTALL + r"""]*\bid=["']__NEXT_DATA__["'][^>]*>(.*?)""", re.DOTALL ) # HLTB publishes no rate limit, so stay well clear of being throttled. @@ -705,11 +707,14 @@ async def _fetch_game_page(self, hltb_id: int) -> dict: await _rate_limiter.acquire() try: - # Request the canonical path directly: the `/game?id=` form answers - # with a redirect, which the shared client does not follow. + # The canonical path, which answers directly today. Redirects are + # followed anyway because a 3xx does not raise, so a hop HLTB added + # later would otherwise read as a page we can no longer parse. The + # client validates every hop against SSRF, redirects included. res = await httpx_client.get( f"{self.base_url}/game/{hltb_id}", headers=self._base_headers(), + follow_redirects=True, timeout=60, ) res.raise_for_status() diff --git a/backend/tests/handler/metadata/test_hltb_handler.py b/backend/tests/handler/metadata/test_hltb_handler.py index 8462468c6..39717d909 100644 --- a/backend/tests/handler/metadata/test_hltb_handler.py +++ b/backend/tests/handler/metadata/test_hltb_handler.py @@ -678,3 +678,74 @@ async def test_transport_failures_report_service_unavailable( assert exc_info.value.status_code == status.HTTP_503_SERVICE_UNAVAILABLE assert "check your internet connection" in exc_info.value.detail + + +@pytest.mark.parametrize( + ("label", "attributes"), + [ + ("plain", 'id="__NEXT_DATA__" type="application/json"'), + ("csp nonce", 'id="__NEXT_DATA__" type="application/json" nonce="r4nd0m"'), + ("reordered", 'type="application/json" id="__NEXT_DATA__"'), + ("single quotes", "id='__NEXT_DATA__' type='application/json'"), + ("crossorigin", 'crossorigin="" id="__NEXT_DATA__" type="application/json"'), + ], +) +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_hydration_tag_is_found_however_it_is_marked_up( + mock_ctx_httpx_client, label, attributes +): + """Markup around the payload is not the contract; the id is. Reporting a + rewrite over an added nonce would be a false alarm.""" + payload = json.dumps( + {"props": {"pageProps": {"game": {"data": {"game": [_game(7169, "Pokémon")]}}}}} + ) + response = MagicMock() + response.status_code = 200 + response.text = f"" + client = MagicMock() + client.get = AsyncMock(return_value=response) + mock_ctx_httpx_client.get.return_value = client + + rom = await _handler().get_rom_by_id(7169) + + assert rom["hltb_id"] == 7169, label + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_a_redirect_is_followed_rather_than_read_as_a_rewrite( + mock_ctx_httpx_client, +): + """`raise_for_status` lets a 3xx through, so an unfollowed hop would reach + the parser as a page with no payload and be blamed on HLTB reshaping it.""" + handler = _handler() + client = MagicMock() + client.get = AsyncMock(return_value=_game_page(_game(7169, "Pokémon"))) + mock_ctx_httpx_client.get.return_value = client + + await handler.get_rom_by_id(7169) + + assert client.get.await_args.kwargs["follow_redirects"] is True + + +@patch("handler.metadata.hltb_handler.HLTB_API_ENABLED", True) +@patch("handler.metadata.hltb_handler.ctx_httpx_client") +async def test_an_unrelated_json_script_is_not_mistaken_for_the_payload( + mock_ctx_httpx_client, +): + """The looser tag match must not start picking up other JSON blocks.""" + handler = _handler() + response = MagicMock() + response.status_code = 200 + response.text = ( + '' + ) + client = MagicMock() + client.get = AsyncMock(return_value=response) + mock_ctx_httpx_client.get.return_value = client + + with pytest.raises(HTTPException) as exc_info: + await handler.get_rom_by_id(7169) + + assert exc_info.value.status_code == status.HTTP_502_BAD_GATEWAY