mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-03-16 12:21:50 +01:00
fix: handle missing versand_s selector for non-commercial accounts (#869)
## Problem `web_check()` delegates to `web_find()`, which raises `TimeoutError` when an element does not exist in the DOM at all — not just when it is hidden. The `versand_s` `<select>` element was removed from the ad posting form for non-commercial (private) accounts on kleinanzeigen.de, causing all ads with `shipping_type=SHIPPING` and no explicit `shipping_options` to fail with: ``` TimeoutError: No HTML element found using XPath '//select[contains(@id, ".versand_s")]' within N seconds. ``` This affects the `else` branch in `__set_shipping()` where `web_check(By.XPATH, special_shipping_selector, Is.DISPLAYED)` is called without handling the case where the element is entirely absent. ## Fix - Wrap the commercial-account `versand_s` check in `try/except TimeoutError` so that non-commercial accounts (where the element no longer exists) gracefully fall through to the dialog-based shipping flow. - Use `short_timeout` (quick_dom) instead of the default timeout to avoid waiting the full timeout duration for an element that will never appear. ## Test plan - [ ] Publish an ad with `shipping_type: SHIPPING` and no `shipping_options` from a private (non-commercial) account - [ ] Verify the bot correctly falls through to the "Versandmethoden auswählen" dialog - [ ] Verify commercial accounts with the `versand_s` dropdown still work as before <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved shipping selection flow with a guarded probe and reliable fallback so non-commercial accounts and UI variants continue to work when certain elements are absent. * **Tests** * Added unit tests covering shipping selector timeout/fallback behavior and URL construction to prevent regressions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Liermann Torsten - Hays <liermann.hays@partner.akdb.de> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -2033,11 +2033,17 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
await self.__set_shipping_options(ad_cfg, mode)
|
||||
else:
|
||||
special_shipping_selector = '//select[contains(@id, ".versand_s")]'
|
||||
if await self.web_check(By.XPATH, special_shipping_selector, Is.DISPLAYED):
|
||||
# try to set special attribute selector (then we have a commercial account)
|
||||
is_commercial_shipping = False
|
||||
try:
|
||||
has_commercial_selector = await self.web_check(By.XPATH, special_shipping_selector, Is.DISPLAYED, timeout = short_timeout)
|
||||
except TimeoutError:
|
||||
# Element does not exist in DOM (non-commercial account or UI change); fall through to dialog-based shipping.
|
||||
has_commercial_selector = False
|
||||
if has_commercial_selector:
|
||||
shipping_value = "ja" if ad_cfg.shipping_type == "SHIPPING" else "nein"
|
||||
await self.web_select(By.XPATH, special_shipping_selector, shipping_value)
|
||||
else:
|
||||
is_commercial_shipping = True
|
||||
if not is_commercial_shipping:
|
||||
try:
|
||||
# no options. only costs. Set custom shipping cost
|
||||
await self.web_click(By.XPATH, '//button//span[contains(., "Versandmethoden auswählen")]')
|
||||
|
||||
@@ -1817,6 +1817,84 @@ class TestKleinanzeigenBotShippingOptions:
|
||||
mock_set_condition.assert_called_once_with("67890") # Converted to string
|
||||
|
||||
|
||||
class TestShippingSelectorTimeout:
|
||||
"""Regression tests for commercial shipping selector (versand_s) timeout handling.
|
||||
|
||||
Ensures that TimeoutError from web_check (element absent) is caught gracefully,
|
||||
while TimeoutError from web_select (element found but interaction fails) propagates.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_versand_s_falls_back_to_dialog(self, test_bot:KleinanzeigenBot, base_ad_config:dict[str, Any]) -> None:
|
||||
"""When versand_s selector is absent, web_check raises TimeoutError and the bot falls through to dialog-based shipping."""
|
||||
ad_cfg = Ad.model_validate(base_ad_config | {"shipping_type": "SHIPPING"})
|
||||
|
||||
with (
|
||||
patch.object(test_bot, "web_check", new_callable = AsyncMock, side_effect = TimeoutError("element not found")) as mock_check,
|
||||
patch.object(test_bot, "web_select", new_callable = AsyncMock) as mock_select,
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock) as mock_click,
|
||||
patch.object(test_bot, "web_find", new_callable = AsyncMock),
|
||||
patch.object(test_bot, "web_input", new_callable = AsyncMock),
|
||||
):
|
||||
await getattr(test_bot, "_KleinanzeigenBot__set_shipping")(ad_cfg)
|
||||
|
||||
# Probe must have been awaited with quick_dom timeout
|
||||
mock_check.assert_awaited_once()
|
||||
assert mock_check.await_args is not None
|
||||
assert mock_check.await_args.kwargs["timeout"] == test_bot._timeout("quick_dom")
|
||||
|
||||
# web_select must NOT have been called with versand_s (commercial path was skipped)
|
||||
for call in mock_select.call_args_list:
|
||||
assert "versand_s" not in str(call), "web_select should not be called for versand_s when element is absent"
|
||||
|
||||
# Dialog-based fallback should have been triggered (click on "Versandmethoden auswählen")
|
||||
clicked_selectors = [str(c) for c in mock_click.call_args_list]
|
||||
assert any("Versandmethoden" in s for s in clicked_selectors), \
|
||||
"Expected dialog-based shipping fallback when versand_s is absent"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_visible_versand_s_uses_commercial_select(self, test_bot:KleinanzeigenBot, base_ad_config:dict[str, Any]) -> None:
|
||||
"""When versand_s selector is present, web_check succeeds and web_select sets the value."""
|
||||
ad_cfg = Ad.model_validate(base_ad_config | {"shipping_type": "SHIPPING"})
|
||||
|
||||
with (
|
||||
patch.object(test_bot, "web_check", new_callable = AsyncMock, return_value = True) as mock_check,
|
||||
patch.object(test_bot, "web_select", new_callable = AsyncMock) as mock_select,
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock) as mock_click,
|
||||
):
|
||||
await getattr(test_bot, "_KleinanzeigenBot__set_shipping")(ad_cfg)
|
||||
|
||||
# Probe must have been awaited with quick_dom timeout
|
||||
mock_check.assert_awaited_once()
|
||||
assert mock_check.await_args is not None
|
||||
assert mock_check.await_args.kwargs["timeout"] == test_bot._timeout("quick_dom")
|
||||
|
||||
# web_select must have been awaited with versand_s and "ja" (SHIPPING)
|
||||
mock_select.assert_awaited_once_with(By.XPATH, '//select[contains(@id, ".versand_s")]', "ja")
|
||||
|
||||
# Dialog-based fallback should NOT have been triggered
|
||||
clicked_selectors = [str(c) for c in mock_click.call_args_list]
|
||||
assert not any("Versandmethoden" in s for s in clicked_selectors), \
|
||||
"Dialog-based shipping should not be triggered when versand_s is present"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_select_timeout_propagates_after_successful_probe(self, test_bot:KleinanzeigenBot, base_ad_config:dict[str, Any]) -> None:
|
||||
"""When web_check succeeds but web_select raises TimeoutError, the error must propagate (not be swallowed)."""
|
||||
ad_cfg = Ad.model_validate(base_ad_config | {"shipping_type": "SHIPPING"})
|
||||
|
||||
with (
|
||||
patch.object(test_bot, "web_check", new_callable = AsyncMock, return_value = True) as mock_check,
|
||||
patch.object(test_bot, "web_select", new_callable = AsyncMock, side_effect = TimeoutError("select timed out")),
|
||||
pytest.raises(TimeoutError, match = "select timed out"),
|
||||
):
|
||||
await getattr(test_bot, "_KleinanzeigenBot__set_shipping")(ad_cfg)
|
||||
|
||||
# Probe must have been awaited with quick_dom timeout
|
||||
mock_check.assert_awaited_once()
|
||||
assert mock_check.await_args is not None
|
||||
assert mock_check.await_args.kwargs["timeout"] == test_bot._timeout("quick_dom")
|
||||
|
||||
|
||||
class TestKleinanzeigenBotUrlConstruction:
|
||||
"""Tests for URL construction functionality."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user