From 03dbd54e85e37fc00684143eff6c4d656f2a9d01 Mon Sep 17 00:00:00 2001
From: Torsten Liermann <mist@liermann.biz>
Date: Fri, 13 Mar 2026 21:02:05 +0100
Subject: [PATCH] fix: handle missing versand_s selector for non-commercial
 accounts (#869)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 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>
---
 src/kleinanzeigen_bot/__init__.py | 12 +++--
 tests/unit/test_init.py           | 78 +++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/src/kleinanzeigen_bot/__init__.py b/src/kleinanzeigen_bot/__init__.py
index d5a4ee3..d669a22 100644
--- a/src/kleinanzeigen_bot/__init__.py
+++ b/src/kleinanzeigen_bot/__init__.py
@@ -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")]')
diff --git a/tests/unit/test_init.py b/tests/unit/test_init.py
index 6f5da65..8432e0a 100644
--- a/tests/unit/test_init.py
+++ b/tests/unit/test_init.py
@@ -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."""