From c6843a49c014e1a4016a6f0740240f1a89daee2e Mon Sep 17 00:00:00 2001 From: Alex Strutsysnkyi Date: Sun, 21 Jun 2026 20:16:52 +0200 Subject: [PATCH] fix: avoid TypeError abort when web_request gets a non-dict response (#1162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ℹī¸ Description When publishing a batch of ads, a single transient page-load failure could crash the **entire** run with an unhandled `TypeError`. During the pre-publish delete step, `delete_flow.delete_ad` calls `web_request`, which runs a `fetch(...)` via `web_execute`. When the page context is torn down mid-navigation (the same condition that surfaces as *"Page did not finish loading within 30.0 seconds"*), `web_execute` returns a CDP `ExceptionDetails` object (or `None`) instead of the expected response dict. `web_request` then does `response["statusCode"]`, raising: ``` TypeError: [ExceptionDetails] object is not subscriptable File ".../utils/web_scraping_mixin.py", line 1537, in web_request response["statusCode"] in valid_response_codes, ``` This `TypeError` is **not** caught by the publish retry loop (which handles `TimeoutError` / `ProtocolException`), so it propagates all the way up and aborts the whole batch — every remaining ad goes unpublished. Observed live while publishing ~25 ads: 9 published successfully, two hit transient load timeouts, and the next `delete_ad` request crashed the process. - Link to the related issue(s): Issue # ## 📋 Changes Summary - `web_request`: guard the response — if it is not a `dict` containing `statusCode`, raise a `ProtocolException` (already imported) instead of blindly subscripting it. - `ProtocolException` is already handled by the publish retry/skip logic, so a single transient failure now retries and then skips that one ad, letting the rest of the batch continue. - Add a parametrized regression test in `test_web_scraping_mixin.py` covering `None` / non-dict / missing-`statusCode` responses. ### ⚙ī¸ Type of Change - [x] 🐞 Bug fix (non-breaking change which fixes an issue) - [ ] ✨ New feature (adds new functionality without breaking existing usage) - [ ] đŸ’Ĩ Breaking change (changes that might break existing user setups, scripts, or configurations) ## ✅ Checklist - [x] I have reviewed my changes to ensure they meet the project's standards. - [x] I have tested my changes and ensured the affected tests pass (`pytest tests/unit/test_web_scraping_mixin.py -k web_request` → 7 passed). - [ ] I have formatted the code (`pdm run format`). - [ ] I have verified that linting passes (`pdm run lint`). - [ ] I have updated documentation where necessary (n/a). By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. ## Summary by CodeRabbit * **Bug Fixes** * Enhanced validation for network operation responses with improved error messaging for unexpected situations, preventing hidden failures and providing clearer diagnostics when issues occur. * **Tests** * Extended test coverage to comprehensively validate error handling across various edge cases and unexpected response scenarios in network operations. --- src/kleinanzeigen_bot/utils/web_scraping_mixin.py | 8 ++++++++ tests/unit/test_web_scraping_mixin.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/kleinanzeigen_bot/utils/web_scraping_mixin.py b/src/kleinanzeigen_bot/utils/web_scraping_mixin.py index 9d979a8..e6bf138 100644 --- a/src/kleinanzeigen_bot/utils/web_scraping_mixin.py +++ b/src/kleinanzeigen_bot/utils/web_scraping_mixin.py @@ -1533,6 +1533,14 @@ class WebScrapingMixin: # noqa: PLR0904 """) if isinstance(valid_response_codes, int): valid_response_codes = [valid_response_codes] + # web_execute may return a CDP ExceptionDetails (or None) instead of the + # expected dict when the page context is torn down mid-navigation. Treat + # that as a transient protocol failure so the publish retry loop skips the + # ad instead of crashing the whole batch with a TypeError. + if not isinstance(response, dict) or "statusCode" not in response: + raise ProtocolException( + f"Unexpected response for HTTP {method} to {url}: {response!r}" + ) ensure( response["statusCode"] in valid_response_codes, f'Invalid response "{response["statusCode"]} {response["statusMessage"]}" received for HTTP {method} to {url}', diff --git a/tests/unit/test_web_scraping_mixin.py b/tests/unit/test_web_scraping_mixin.py index 02badf0..5f90c4e 100644 --- a/tests/unit/test_web_scraping_mixin.py +++ b/tests/unit/test_web_scraping_mixin.py @@ -18,6 +18,7 @@ from unittest.mock import ANY, AsyncMock, MagicMock, Mock, mock_open, patch import nodriver import psutil import pytest +from nodriver.core.connection import ProtocolException from nodriver.core.element import Element from nodriver.core.tab import Tab as Page @@ -448,6 +449,19 @@ class TestWebScrapingErrorHandling: with pytest.raises(Exception, match = "Network error"): await web_scraper.web_request("https://example.com") + @pytest.mark.asyncio + @pytest.mark.parametrize("bad_response", [None, "boom", 42, ["not", "a", "dict"], {"foo": "bar"}]) + async def test_web_request_non_dict_response_raises_protocol_exception( + self, web_scraper:WebScrapingMixin, mock_page:TrulyAwaitableMockPage, bad_response:object, + ) -> None: + """A non-dict / missing-statusCode response (e.g. CDP ExceptionDetails when the page + context is torn down mid-navigation) must raise a catchable ProtocolException rather + than a TypeError that would abort an entire publish batch.""" + mock_page.evaluate.return_value = bad_response + + with pytest.raises(ProtocolException): + await web_scraper.web_request("https://example.com") + @pytest.mark.asyncio async def test_web_check_element_not_found(self, web_scraper:WebScrapingMixin, mock_page:TrulyAwaitableMockPage) -> None: """Test element not found error in web_check."""