Differentiate mapping field inspection and read failures

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

Author: cursoragent

hyperbrowser/tools/__init__.py

@@ -198,10 +198,13 @@ def _read_tool_response_data(response: Any, *, tool_name: str) -> Any:
             raise HyperbrowserError(f"{tool_name} response must include 'data'")
         try:
             return response["data"]
-        except KeyError:
-            raise HyperbrowserError(f"{tool_name} response must include 'data'")
         except HyperbrowserError:
             raise
+        except KeyError as exc:
+            raise HyperbrowserError(
+                f"Failed to read {tool_name} response data",
+                original_error=exc,
+            ) from exc
         except Exception as exc:
             raise HyperbrowserError(
                 f"Failed to read {tool_name} response data",
@@ -230,11 +233,25 @@ def _read_optional_tool_response_field(
         return ""
     if isinstance(response_data, MappingABC):
         try:
-            field_value = response_data[field_name]
-        except KeyError:
+            has_field = field_name in response_data
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                f"Failed to inspect {tool_name} response field '{field_name}'",
+                original_error=exc,
+            ) from exc
+        if not has_field:
             return ""
+        try:
+            field_value = response_data[field_name]
         except HyperbrowserError:
             raise
+        except KeyError as exc:
+            raise HyperbrowserError(
+                f"Failed to read {tool_name} response field '{field_name}'",
+                original_error=exc,
+            ) from exc
         except Exception as exc:
             raise HyperbrowserError(
                 f"Failed to read {tool_name} response field '{field_name}'",
@@ -265,11 +282,25 @@ def _read_optional_tool_response_field(
 def _read_crawl_page_field(page: Any, *, field_name: str, page_index: int) -> Any:
     if isinstance(page, MappingABC):
         try:
-            return page[field_name]
-        except KeyError:
+            has_field = field_name in page
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                f"Failed to inspect crawl tool page field '{field_name}' at index {page_index}",
+                original_error=exc,
+            ) from exc
+        if not has_field:
             return None
+        try:
+            return page[field_name]
         except HyperbrowserError:
             raise
+        except KeyError as exc:
+            raise HyperbrowserError(
+                f"Failed to read crawl tool page field '{field_name}' at index {page_index}",
+                original_error=exc,
+            ) from exc
         except Exception as exc:
             raise HyperbrowserError(
                 f"Failed to read crawl tool page field '{field_name}' at index {page_index}",

tests/test_tools_response_handling.py

@@ -169,7 +169,7 @@ def __getitem__(self, key: str) -> object:
     assert exc_info.value.original_error is not None
 
 
-def test_scrape_tool_rejects_mapping_responses_missing_data_on_lookup():
+def test_scrape_tool_wraps_mapping_responses_keyerrors_on_data_lookup():
     class _InconsistentResponse(Mapping[str, object]):
         def __iter__(self):
             yield "data"
@@ -187,10 +187,12 @@ def __getitem__(self, key: str) -> object:
     client = _SyncScrapeClient(_InconsistentResponse())  # type: ignore[arg-type]
 
     with pytest.raises(
-        HyperbrowserError, match="scrape tool response must include 'data'"
-    ):
+        HyperbrowserError, match="Failed to read scrape tool response data"
+    ) as exc_info:
         WebsiteScrapeTool.runnable(client, {"url": "https://example.com"})
 
+    assert exc_info.value.original_error is not None
+
 
 def test_scrape_tool_wraps_mapping_response_data_inspection_failures():
     class _BrokenContainsResponse(Mapping[str, object]):
@@ -343,6 +345,9 @@ def __iter__(self):
         def __len__(self) -> int:
             return 1
 
+        def __contains__(self, key: object) -> bool:
+            return key == "markdown"
+
         def __getitem__(self, key: str) -> object:
             _ = key
             raise RuntimeError("cannot read mapping field")
@@ -358,6 +363,59 @@ def __getitem__(self, key: str) -> object:
     assert exc_info.value.original_error is not None
 
 
+def test_scrape_tool_wraps_mapping_field_keyerrors_after_membership_check():
+    class _InconsistentMapping(Mapping[str, object]):
+        def __iter__(self):
+            yield "markdown"
+
+        def __len__(self) -> int:
+            return 1
+
+        def __contains__(self, key: object) -> bool:
+            return key == "markdown"
+
+        def __getitem__(self, key: str) -> object:
+            _ = key
+            raise KeyError("markdown")
+
+    client = _SyncScrapeClient(_Response(data=_InconsistentMapping()))
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to read scrape tool response field 'markdown'",
+    ) as exc_info:
+        WebsiteScrapeTool.runnable(client, {"url": "https://example.com"})
+
+    assert exc_info.value.original_error is not None
+
+
+def test_scrape_tool_wraps_mapping_field_inspection_failures():
+    class _BrokenContainsMapping(Mapping[str, object]):
+        def __iter__(self):
+            yield "markdown"
+
+        def __len__(self) -> int:
+            return 1
+
+        def __contains__(self, key: object) -> bool:
+            _ = key
+            raise RuntimeError("cannot inspect markdown key")
+
+        def __getitem__(self, key: str) -> object:
+            _ = key
+            return "ignored"
+
+    client = _SyncScrapeClient(_Response(data=_BrokenContainsMapping()))
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to inspect scrape tool response field 'markdown'",
+    ) as exc_info:
+        WebsiteScrapeTool.runnable(client, {"url": "https://example.com"})
+
+    assert exc_info.value.original_error is not None
+
+
 def test_screenshot_tool_rejects_non_string_screenshot_field():
     client = _SyncScrapeClient(_Response(data=SimpleNamespace(screenshot=123)))
 
@@ -454,6 +512,9 @@ def __iter__(self):
         def __len__(self) -> int:
             return 1
 
+        def __contains__(self, key: object) -> bool:
+            return key == "markdown"
+
         def __getitem__(self, key: str) -> object:
             _ = key
             raise RuntimeError("cannot read page field")
@@ -469,6 +530,59 @@ def __getitem__(self, key: str) -> object:
     assert exc_info.value.original_error is not None
 
 
+def test_crawl_tool_wraps_mapping_page_keyerrors_after_membership_check():
+    class _InconsistentPage(Mapping[str, object]):
+        def __iter__(self):
+            yield "markdown"
+
+        def __len__(self) -> int:
+            return 1
+
+        def __contains__(self, key: object) -> bool:
+            return key == "markdown"
+
+        def __getitem__(self, key: str) -> object:
+            _ = key
+            raise KeyError("markdown")
+
+    client = _SyncCrawlClient(_Response(data=[_InconsistentPage()]))
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to read crawl tool page field 'markdown' at index 0",
+    ) as exc_info:
+        WebsiteCrawlTool.runnable(client, {"url": "https://example.com"})
+
+    assert exc_info.value.original_error is not None
+
+
+def test_crawl_tool_wraps_mapping_page_inspection_failures():
+    class _BrokenContainsPage(Mapping[str, object]):
+        def __iter__(self):
+            yield "markdown"
+
+        def __len__(self) -> int:
+            return 1
+
+        def __contains__(self, key: object) -> bool:
+            _ = key
+            raise RuntimeError("cannot inspect markdown key")
+
+        def __getitem__(self, key: str) -> object:
+            _ = key
+            return "ignored"
+
+    client = _SyncCrawlClient(_Response(data=[_BrokenContainsPage()]))
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to inspect crawl tool page field 'markdown' at index 0",
+    ) as exc_info:
+        WebsiteCrawlTool.runnable(client, {"url": "https://example.com"})
+
+    assert exc_info.value.original_error is not None
+
+
 def test_crawl_tool_rejects_non_string_page_urls():
     client = _SyncCrawlClient(
         _Response(data=[SimpleNamespace(url=42, markdown="body")])