steam_backlog_enforcer: fix library_hider crash on invalid AppIDs + improve HLTB hour extraction

- library_hider.py: add safeHide(ids) JS helper that binary-bisects on failure
  to skip problematic DLC/tool IDs without blocking the entire hide pass
- library_hider.py: increase CDP timeout 30s -> 120s; extract richer CDP error
  details from exceptionDetails/exception.description
- _hltb_detail.py: rewrite _extract_base_leisure_hours() to pick the maximum
  (slowest) time across all platform comp_high values and *_h fields; add
  _platform_comp_high_candidates() helper
This commit is contained in:
Krzysztof kuhy Rudnicki 2026-05-22 15:59:18 +02:00
parent dd3191d961
commit 61a9e5dc3c
5 changed files with 216 additions and 12 deletions

View File

@ -0,0 +1,16 @@
{
"title": "Fix Steam library hider crash on invalid AppIDs",
"objective": "hide_other_games() crashes with RuntimeError 'JS evaluation error: Unknown JS error' because SetAppsAsHidden() throws in Steam's AddApps internals when fed AppIDs that are not recognised game entries (e.g. DLC/tool IDs returned by get_all_owned_app_ids via include_played_free_games). The fix makes the JS resilient via a safeHide() binary-search fallback that silently skips rejected IDs, and improves error reporting so future JS failures show the real exception message.",
"acceptance_criteria": [
"hide_other_games() completes without RuntimeError when owned list contains invalid AppIDs",
"Valid game IDs are still hidden correctly",
"Invalid/unrecognised AppIDs are silently skipped (not fatal)",
"CDP exception messages include the actual JS error text instead of 'Unknown JS error'",
"All 590 existing tests continue to pass at 100% coverage"
],
"out_of_scope": [
"Filtering invalid AppIDs on the Python side before passing to JS",
"Changing which AppIDs get_all_owned_app_ids() returns"
],
"verifier": "python -m pytest python_pkg/steam_backlog_enforcer/tests/ -x -q && pre-commit run --files python_pkg/steam_backlog_enforcer/library_hider.py"
}

View File

@ -0,0 +1,37 @@
{
"intent": "Fix RuntimeError 'JS evaluation error: Unknown JS error' when hide_other_games() is called with owned game IDs that include invalid AppIDs (DLC/tools returned by Steam API include_played_free_games).",
"scope": [
"python_pkg/steam_backlog_enforcer/library_hider.py",
"No behavioral change for valid game IDs; invalid IDs are silently skipped"
],
"changes": [
"Replace bare SetAppsAsHidden() call with safeHide() recursive binary-search fallback that skips IDs rejected by Steam's AddApps internals",
"Improve _cdp_result_value() to extract actual JS error from exceptionDetails (exception.description / text) instead of always reporting 'Unknown JS error'",
"Increase _CDP_TIMEOUT from 30s to 120s to accommodate extra fallback calls when problematic IDs trigger individual-retry path"
],
"verification": [
{
"command": "python -c \"from python_pkg.steam_backlog_enforcer.library_hider import _evaluate_js, _cdp_result_value; ...safeHide test...\"",
"result": "pass",
"evidence": "Result: {\"hidden\":3,\"total\":6} - safeHide correctly hid 3 real IDs and silently skipped 3 fake/invalid IDs without throwing"
},
{
"command": "python -m pytest python_pkg/steam_backlog_enforcer/tests/ -x -q --tb=short",
"result": "pass",
"evidence": "590 passed, 0 failures, 100% coverage on steam_backlog_enforcer modules"
},
{
"command": "pre-commit run --files python_pkg/steam_backlog_enforcer/library_hider.py",
"result": "pass",
"evidence": "ruff, mypy, pylint, bandit all passed; only contract artifact hooks pending"
}
],
"risks": [
"safeHide silently skips problematic IDs instead of surfacing them; in theory a valid game could slip through if AddApps has a transient error — second pass via visibleApps would catch it",
"120s CDP timeout may mask genuinely hung JS evaluations"
],
"rollback": [
"git revert the library_hider.py changes to restore bare SetAppsAsHidden() call and 30s timeout",
"Verify tests still pass after rollback"
]
}

View File

@ -56,8 +56,32 @@ def _as_positive_int(value: object) -> int:
return 0 return 0
def _platform_comp_high_candidates(game_data: dict[str, Any]) -> list[int]:
"""Collect positive ``comp_high`` values from ``platformData`` entries."""
platform_data = game_data.get("platformData", [])
if not isinstance(platform_data, list):
return []
candidates = []
for entry in platform_data:
if isinstance(entry, dict):
v = _as_positive_int(entry.get("comp_high", 0))
if v > 0:
candidates.append(v)
return candidates
def _extract_base_leisure_hours(game_data: dict[str, Any]) -> float: def _extract_base_leisure_hours(game_data: dict[str, Any]) -> float:
"""Extract base-game leisure hours from game detail data.""" """Extract base-game leisure hours from game detail data.
Returns the highest (slowest) time to beat across all play styles.
Candidates considered:
1. ``comp_high`` from each entry in ``platformData`` the per-platform
slowest individual submission displayed on the HLTB page.
2. The ``_h`` (leisure/high) fields from ``game[0]``:
``comp_main_h``, ``comp_plus_h``, ``comp_100_h``, ``comp_all_h``.
3. Falls back to average times: ``comp_main``, ``comp_plus``, ``comp_100``.
"""
games = game_data.get("game", []) games = game_data.get("game", [])
if not isinstance(games, list) or not games: if not isinstance(games, list) or not games:
return -1 return -1
@ -65,9 +89,25 @@ def _extract_base_leisure_hours(game_data: dict[str, Any]) -> float:
return -1 return -1
base = games[0] base = games[0]
leisure_s = _as_positive_int(base.get("comp_100_h", 0)) candidates = _platform_comp_high_candidates(game_data)
# 2. Leisure/high fields from the game record
for field in ("comp_main_h", "comp_plus_h", "comp_100_h", "comp_all_h"):
v = _as_positive_int(base.get(field, 0))
if v > 0:
candidates.append(v)
leisure_s = max(candidates) if candidates else 0
# 3. Fallback: average completion times
if leisure_s <= 0: if leisure_s <= 0:
leisure_s = _as_positive_int(base.get("comp_100", 0)) avg_candidates = [
_as_positive_int(base.get("comp_main", 0)),
_as_positive_int(base.get("comp_plus", 0)),
_as_positive_int(base.get("comp_100", 0)),
]
leisure_s = max(avg_candidates)
if leisure_s <= 0: if leisure_s <= 0:
return -1 return -1
@ -100,9 +140,9 @@ def _extract_dlc_relationships(game_data: dict[str, Any]) -> list[tuple[int, flo
def _extract_leisure_hours(game_data: dict[str, Any]) -> float: def _extract_leisure_hours(game_data: dict[str, Any]) -> float:
"""Compute total leisure hours: base game + all DLCs. """Compute total leisure hours: base game + all DLCs.
Uses ``comp_100_h`` (leisure completionist) from the game detail page. Uses the highest (slowest) time across ``platformData comp_high`` and
Falls back to ``comp_100`` (average completionist) if leisure unavailable. leisure ``_h`` fields from ``game[0]``. Falls back to average completion
Also sums leisure time from any DLC listed in ``relationships``. times. Also sums leisure time from any DLC listed in ``relationships``.
""" """
base_hours = _extract_base_leisure_hours(game_data) base_hours = _extract_base_leisure_hours(game_data)
if base_hours <= 0: if base_hours <= 0:

View File

@ -31,7 +31,7 @@ import websockets
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
_CDP_PORT = 8080 _CDP_PORT = 8080
_CDP_TIMEOUT = 30 _CDP_TIMEOUT = 120
_STEAM_STARTUP_WAIT = 45 _STEAM_STARTUP_WAIT = 45
@ -85,9 +85,18 @@ def _evaluate_js(expression: str) -> dict:
def _cdp_result_value(result: dict) -> str: def _cdp_result_value(result: dict) -> str:
"""Extract the return value from a CDP Runtime.evaluate response.""" """Extract the return value from a CDP Runtime.evaluate response."""
inner = result.get("result", {}).get("result", {}) outer = result.get("result", {})
if "exceptionDetails" in result.get("result", {}): inner = outer.get("result", {})
desc = inner.get("description", "Unknown JS error") if "exceptionDetails" in outer:
exc_details = outer["exceptionDetails"]
exc = exc_details.get("exception", {})
desc = (
inner.get("description")
or exc.get("description")
or exc_details.get("text")
or repr(exc_details)
)
logger.debug("CDP exception details: %s", exc_details)
msg = f"JS evaluation error: {desc}" msg = f"JS evaluation error: {desc}"
raise RuntimeError(msg) raise RuntimeError(msg)
value: str = inner.get("value", "") value: str = inner.get("value", "")
@ -251,6 +260,19 @@ def hide_other_games(
const maxPasses = {_MAX_HIDE_PASSES}; const maxPasses = {_MAX_HIDE_PASSES};
const batchSize = {_HIDE_BATCH_SIZE}; const batchSize = {_HIDE_BATCH_SIZE};
async function safeHide(ids) {{
if (ids.length === 0) return 0;
try {{
await collectionStore.SetAppsAsHidden(ids, true);
return ids.length;
}} catch(e) {{
if (ids.length === 1) return 0;
const mid = Math.floor(ids.length / 2);
return (await safeHide(ids.slice(0, mid))) +
(await safeHide(ids.slice(mid)));
}}
}}
for (let pass = 0; pass < maxPasses; pass++) {{ for (let pass = 0; pass < maxPasses; pass++) {{
let visible = coll && coll.visibleApps let visible = coll && coll.visibleApps
? coll.visibleApps.map(a => a.appid).filter(id => id !== allowed) ? coll.visibleApps.map(a => a.appid).filter(id => id !== allowed)
@ -267,8 +289,7 @@ def hide_other_games(
for (let i = 0; i < visible.length; i += batchSize) {{ for (let i = 0; i < visible.length; i += batchSize) {{
const batch = visible.slice(i, i + batchSize); const batch = visible.slice(i, i + batchSize);
await collectionStore.SetAppsAsHidden(batch, true); totalHidden += await safeHide(batch);
totalHidden += batch.length;
}} }}
await new Promise(r => setTimeout(r, {_SETTLE_DELAY_MS})); await new Promise(r => setTimeout(r, {_SETTLE_DELAY_MS}));

View File

@ -36,6 +36,70 @@ class TestInternalHelpers:
data: dict[str, Any] = {"game": [123]} data: dict[str, Any] = {"game": [123]}
assert _extract_base_leisure_hours(data) == -1 assert _extract_base_leisure_hours(data) == -1
def test_extract_base_leisure_platform_data_comp_high_is_max(self) -> None:
data: dict[str, Any] = {
"game": [{"comp_100_h": 16063}],
"platformData": [{"platform": "PC", "comp_high": 23760}],
}
assert _extract_base_leisure_hours(data) == round(23760 / 3600, 2)
def test_extract_base_leisure_h_field_exceeds_platform_comp_high(self) -> None:
data: dict[str, Any] = {
"game": [{"comp_100_h": 25000}],
"platformData": [{"platform": "PC", "comp_high": 23760}],
}
assert _extract_base_leisure_hours(data) == round(25000 / 3600, 2)
def test_extract_base_leisure_max_of_multiple_platforms(self) -> None:
data: dict[str, Any] = {
"game": [{}],
"platformData": [
{"platform": "PC", "comp_high": 23760},
{"platform": "Switch", "comp_high": 18000},
],
}
assert _extract_base_leisure_hours(data) == round(23760 / 3600, 2)
def test_extract_base_leisure_platform_data_not_list(self) -> None:
data: dict[str, Any] = {
"game": [{"comp_100_h": 16063}],
"platformData": "not_a_list",
}
assert _extract_base_leisure_hours(data) == round(16063 / 3600, 2)
def test_extract_base_leisure_platform_non_dict_entry_skipped(self) -> None:
data: dict[str, Any] = {
"game": [{"comp_100_h": 16063}],
"platformData": ["bad", {"platform": "PC", "comp_high": 23760}],
}
assert _extract_base_leisure_hours(data) == round(23760 / 3600, 2)
def test_extract_base_leisure_platform_comp_high_zero_skipped(self) -> None:
data: dict[str, Any] = {
"game": [{"comp_100_h": 16063}],
"platformData": [{"platform": "PC", "comp_high": 0}],
}
assert _extract_base_leisure_hours(data) == round(16063 / 3600, 2)
def test_extract_base_leisure_max_of_h_fields(self) -> None:
data: dict[str, Any] = {
"game": [
{
"comp_main_h": 14951,
"comp_plus_h": 17957,
"comp_100_h": 16063,
"comp_all_h": 17959,
}
],
}
assert _extract_base_leisure_hours(data) == round(17959 / 3600, 2)
def test_extract_base_leisure_fallback_to_avg_comp_main(self) -> None:
data: dict[str, Any] = {
"game": [{"comp_main": 10800, "comp_plus": 0, "comp_100": 0}],
}
assert _extract_base_leisure_hours(data) == round(10800 / 3600, 2)
def test_extract_dlc_relationships_skips_non_dict(self) -> None: def test_extract_dlc_relationships_skips_non_dict(self) -> None:
data: dict[str, Any] = { data: dict[str, Any] = {
"relationships": [ "relationships": [
@ -376,3 +440,29 @@ class TestFetchLeisureTimes:
expected = round((21243 + 4075) / 3600, 2) expected = round((21243 + 4075) / 3600, 2)
assert cache[1289310] == expected assert cache[1289310] == expected
assert results[0].completionist_hours == expected assert results[0].completionist_hours == expected
def test_with_explicit_count_comp(self) -> None:
"""Pass a non-None count_comp to cover the False branch of the None check."""
results = [
HLTBResult(
app_id=440,
game_name="TF2",
completionist_hours=50.0,
similarity=1.0,
hltb_game_id=12345,
),
]
game_data: dict[str, Any] = {
"game": [{"comp_100_h": 3600}],
"relationships": [],
}
cache: dict[int, float] = {}
with patch(
"python_pkg.steam_backlog_enforcer._hltb_detail._fetch_detail_one",
new_callable=AsyncMock,
return_value=game_data,
):
asyncio.run(
_fetch_leisure_times(results, cache, {}, None, count_comp={440: 5})
)
assert cache[440] == 1.0