Harden crawl and batch-scrape get param serialization

Co-authored-by: Shri Sukhani <shrisukhani@users.noreply.github.com>

Author: cursoragent

hyperbrowser/client/managers/async_manager/crawl.py

@@ -60,9 +60,20 @@ async def get(
         self, job_id: str, params: Optional[GetCrawlJobParams] = None
     ) -> CrawlJobResponse:
         params_obj = params or GetCrawlJobParams()
+        try:
+            query_params = params_obj.model_dump(exclude_none=True, by_alias=True)
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                "Failed to serialize crawl get params",
+                original_error=exc,
+            ) from exc
+        if type(query_params) is not dict:
+            raise HyperbrowserError("Failed to serialize crawl get params")
         response = await self._client.transport.get(
             self._client._build_url(f"/crawl/{job_id}"),
-            params=params_obj.model_dump(exclude_none=True, by_alias=True),
+            params=query_params,
         )
         return parse_response_model(
             response.data,

hyperbrowser/client/managers/async_manager/scrape.py

@@ -67,9 +67,20 @@ async def get(
         self, job_id: str, params: Optional[GetBatchScrapeJobParams] = None
     ) -> BatchScrapeJobResponse:
         params_obj = params or GetBatchScrapeJobParams()
+        try:
+            query_params = params_obj.model_dump(exclude_none=True, by_alias=True)
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                "Failed to serialize batch scrape get params",
+                original_error=exc,
+            ) from exc
+        if type(query_params) is not dict:
+            raise HyperbrowserError("Failed to serialize batch scrape get params")
         response = await self._client.transport.get(
             self._client._build_url(f"/scrape/batch/{job_id}"),
-            params=params_obj.model_dump(exclude_none=True, by_alias=True),
+            params=query_params,
         )
         return parse_response_model(
             response.data,

hyperbrowser/client/managers/sync_manager/crawl.py

@@ -60,9 +60,20 @@ def get(
         self, job_id: str, params: Optional[GetCrawlJobParams] = None
     ) -> CrawlJobResponse:
         params_obj = params or GetCrawlJobParams()
+        try:
+            query_params = params_obj.model_dump(exclude_none=True, by_alias=True)
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                "Failed to serialize crawl get params",
+                original_error=exc,
+            ) from exc
+        if type(query_params) is not dict:
+            raise HyperbrowserError("Failed to serialize crawl get params")
         response = self._client.transport.get(
             self._client._build_url(f"/crawl/{job_id}"),
-            params=params_obj.model_dump(exclude_none=True, by_alias=True),
+            params=query_params,
         )
         return parse_response_model(
             response.data,

hyperbrowser/client/managers/sync_manager/scrape.py

@@ -65,9 +65,20 @@ def get(
         self, job_id: str, params: Optional[GetBatchScrapeJobParams] = None
     ) -> BatchScrapeJobResponse:
         params_obj = params or GetBatchScrapeJobParams()
+        try:
+            query_params = params_obj.model_dump(exclude_none=True, by_alias=True)
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                "Failed to serialize batch scrape get params",
+                original_error=exc,
+            ) from exc
+        if type(query_params) is not dict:
+            raise HyperbrowserError("Failed to serialize batch scrape get params")
         response = self._client.transport.get(
             self._client._build_url(f"/scrape/batch/{job_id}"),
-            params=params_obj.model_dump(exclude_none=True, by_alias=True),
+            params=query_params,
         )
         return parse_response_model(
             response.data,

tests/test_job_manager_serialization.py

@@ -29,9 +29,13 @@
     ScrapeManager as SyncScrapeManager,
 )
 from hyperbrowser.exceptions import HyperbrowserError
-from hyperbrowser.models.crawl import StartCrawlJobParams
+from hyperbrowser.models.crawl import GetCrawlJobParams, StartCrawlJobParams
 from hyperbrowser.models.extract import StartExtractJobParams
-from hyperbrowser.models.scrape import StartBatchScrapeJobParams, StartScrapeJobParams
+from hyperbrowser.models.scrape import (
+    GetBatchScrapeJobParams,
+    StartBatchScrapeJobParams,
+    StartScrapeJobParams,
+)
 
 
 class _SyncTransport:
@@ -175,6 +179,49 @@ def _build_url(self, path: str) -> str:
     ),
 )
 
+_SyncGetCase = Tuple[
+    str,
+    Type[Any],
+    Type[Any],
+    Callable[[], Any],
+    str,
+]
+_AsyncGetCase = _SyncGetCase
+
+SYNC_GET_CASES: tuple[_SyncGetCase, ...] = (
+    (
+        "batch-scrape-get",
+        SyncBatchScrapeManager,
+        GetBatchScrapeJobParams,
+        lambda: GetBatchScrapeJobParams(page=1),
+        "Failed to serialize batch scrape get params",
+    ),
+    (
+        "crawl-get",
+        SyncCrawlManager,
+        GetCrawlJobParams,
+        lambda: GetCrawlJobParams(page=1),
+        "Failed to serialize crawl get params",
+    ),
+)
+
+ASYNC_GET_CASES: tuple[_AsyncGetCase, ...] = (
+    (
+        "batch-scrape-get",
+        AsyncBatchScrapeManager,
+        GetBatchScrapeJobParams,
+        lambda: GetBatchScrapeJobParams(page=1),
+        "Failed to serialize batch scrape get params",
+    ),
+    (
+        "crawl-get",
+        AsyncCrawlManager,
+        GetCrawlJobParams,
+        lambda: GetCrawlJobParams(page=1),
+        "Failed to serialize crawl get params",
+    ),
+)
+
 
 @pytest.mark.parametrize(
     "_, manager_class, __, build_params, expected_url, expected_payload, ___",
@@ -420,3 +467,179 @@ async def run() -> None:
         assert exc_info.value.original_error is None
 
     asyncio.run(run())
+
+
+@pytest.mark.parametrize(
+    "_, manager_class, params_class, build_params, expected_error",
+    SYNC_GET_CASES,
+    ids=[case[0] for case in SYNC_GET_CASES],
+)
+def test_sync_job_get_wraps_param_serialization_errors(
+    _: str,
+    manager_class: Type[Any],
+    params_class: Type[Any],
+    build_params: Callable[[], Any],
+    expected_error: str,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = manager_class(_SyncClient())
+
+    def _raise_model_dump_error(*args, **kwargs):
+        _ = args
+        _ = kwargs
+        raise RuntimeError("broken model_dump")
+
+    monkeypatch.setattr(params_class, "model_dump", _raise_model_dump_error)
+
+    with pytest.raises(HyperbrowserError, match=expected_error) as exc_info:
+        manager.get("job_123", build_params())
+
+    assert isinstance(exc_info.value.original_error, RuntimeError)
+
+
+@pytest.mark.parametrize(
+    "_, manager_class, params_class, build_params, expected_error",
+    SYNC_GET_CASES,
+    ids=[case[0] for case in SYNC_GET_CASES],
+)
+def test_sync_job_get_preserves_hyperbrowser_serialization_errors(
+    _: str,
+    manager_class: Type[Any],
+    params_class: Type[Any],
+    build_params: Callable[[], Any],
+    expected_error: str,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = manager_class(_SyncClient())
+
+    def _raise_model_dump_error(*args, **kwargs):
+        _ = args
+        _ = kwargs
+        raise HyperbrowserError("custom model_dump failure")
+
+    monkeypatch.setattr(params_class, "model_dump", _raise_model_dump_error)
+
+    with pytest.raises(
+        HyperbrowserError, match="custom model_dump failure"
+    ) as exc_info:
+        manager.get("job_123", build_params())
+
+    assert exc_info.value.original_error is None
+
+
+@pytest.mark.parametrize(
+    "_, manager_class, params_class, build_params, expected_error",
+    SYNC_GET_CASES,
+    ids=[case[0] for case in SYNC_GET_CASES],
+)
+def test_sync_job_get_rejects_non_dict_serialized_params(
+    _: str,
+    manager_class: Type[Any],
+    params_class: Type[Any],
+    build_params: Callable[[], Any],
+    expected_error: str,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = manager_class(_SyncClient())
+
+    monkeypatch.setattr(
+        params_class,
+        "model_dump",
+        lambda *args, **kwargs: MappingProxyType({"page": 1}),
+    )
+
+    with pytest.raises(HyperbrowserError, match=expected_error) as exc_info:
+        manager.get("job_123", build_params())
+
+    assert exc_info.value.original_error is None
+
+
+@pytest.mark.parametrize(
+    "_, manager_class, params_class, build_params, expected_error",
+    ASYNC_GET_CASES,
+    ids=[case[0] for case in ASYNC_GET_CASES],
+)
+def test_async_job_get_wraps_param_serialization_errors(
+    _: str,
+    manager_class: Type[Any],
+    params_class: Type[Any],
+    build_params: Callable[[], Any],
+    expected_error: str,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = manager_class(_AsyncClient())
+
+    def _raise_model_dump_error(*args, **kwargs):
+        _ = args
+        _ = kwargs
+        raise RuntimeError("broken model_dump")
+
+    monkeypatch.setattr(params_class, "model_dump", _raise_model_dump_error)
+
+    async def run() -> None:
+        with pytest.raises(HyperbrowserError, match=expected_error) as exc_info:
+            await manager.get("job_123", build_params())
+        assert isinstance(exc_info.value.original_error, RuntimeError)
+
+    asyncio.run(run())
+
+
+@pytest.mark.parametrize(
+    "_, manager_class, params_class, build_params, expected_error",
+    ASYNC_GET_CASES,
+    ids=[case[0] for case in ASYNC_GET_CASES],
+)
+def test_async_job_get_preserves_hyperbrowser_serialization_errors(
+    _: str,
+    manager_class: Type[Any],
+    params_class: Type[Any],
+    build_params: Callable[[], Any],
+    expected_error: str,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = manager_class(_AsyncClient())
+
+    def _raise_model_dump_error(*args, **kwargs):
+        _ = args
+        _ = kwargs
+        raise HyperbrowserError("custom model_dump failure")
+
+    monkeypatch.setattr(params_class, "model_dump", _raise_model_dump_error)
+
+    async def run() -> None:
+        with pytest.raises(
+            HyperbrowserError, match="custom model_dump failure"
+        ) as exc_info:
+            await manager.get("job_123", build_params())
+        assert exc_info.value.original_error is None
+
+    asyncio.run(run())
+
+
+@pytest.mark.parametrize(
+    "_, manager_class, params_class, build_params, expected_error",
+    ASYNC_GET_CASES,
+    ids=[case[0] for case in ASYNC_GET_CASES],
+)
+def test_async_job_get_rejects_non_dict_serialized_params(
+    _: str,
+    manager_class: Type[Any],
+    params_class: Type[Any],
+    build_params: Callable[[], Any],
+    expected_error: str,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = manager_class(_AsyncClient())
+
+    monkeypatch.setattr(
+        params_class,
+        "model_dump",
+        lambda *args, **kwargs: MappingProxyType({"page": 1}),
+    )
+
+    async def run() -> None:
+        with pytest.raises(HyperbrowserError, match=expected_error) as exc_info:
+            await manager.get("job_123", build_params())
+        assert exc_info.value.original_error is None
+
+    asyncio.run(run())