Use shared binary file opener in upload managers

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

Author: cursoragent

CONTRIBUTING.md

@@ -77,6 +77,7 @@ This runs lint, format checks, compile checks, tests, and package build.
 - Prefer deterministic unit tests over network-dependent tests.
 - Preserve architectural guardrails with focused tests. Current guard suites include:
   - `tests/test_architecture_marker_usage.py` (architecture marker coverage across guard modules),
+  - `tests/test_binary_file_open_helper_usage.py` (shared binary file open helper usage enforcement),
   - `tests/test_ci_workflow_quality_gates.py` (CI guard-stage + make-target enforcement),
   - `tests/test_computer_action_endpoint_helper_usage.py` (computer-action endpoint-normalization helper usage enforcement),
   - `tests/test_contributing_architecture_guard_listing.py` (`CONTRIBUTING.md` architecture-guard inventory completeness enforcement),

hyperbrowser/client/file_utils.py

@@ -1,6 +1,7 @@
 import os
+from contextlib import contextmanager
 from os import PathLike
-from typing import Union
+from typing import BinaryIO, Iterator, Union
 
 from hyperbrowser.exceptions import HyperbrowserError
 from hyperbrowser.type_utils import is_plain_string
@@ -125,3 +126,38 @@ def ensure_existing_file_path(
     if not is_file:
         raise HyperbrowserError(not_file_message)
     return normalized_path
+
+
+@contextmanager
+def open_binary_file(
+    file_path: Union[str, PathLike[str]],
+    *,
+    open_error_message: str,
+) -> Iterator[BinaryIO]:
+    _validate_error_message_text(
+        open_error_message,
+        field_name="open_error_message",
+    )
+    try:
+        normalized_path = os.fspath(file_path)
+    except HyperbrowserError:
+        raise
+    except TypeError as exc:
+        raise HyperbrowserError(
+            "file_path must be a string or os.PathLike object",
+            original_error=exc,
+        ) from exc
+    except Exception as exc:
+        raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
+    if not is_plain_string(normalized_path):
+        raise HyperbrowserError("file_path must resolve to a string path")
+    try:
+        with open(normalized_path, "rb") as file_obj:
+            yield file_obj
+    except HyperbrowserError:
+        raise
+    except Exception as exc:
+        raise HyperbrowserError(
+            open_error_message,
+            original_error=exc,
+        ) from exc

hyperbrowser/client/managers/async_manager/extension.py

@@ -1,6 +1,6 @@
 from typing import List
 
-from hyperbrowser.exceptions import HyperbrowserError
+from ...file_utils import open_binary_file
 from ..extension_create_utils import normalize_extension_create_input
 from ..extension_utils import parse_extension_list_response_data
 from ..response_utils import parse_response_model
@@ -14,18 +14,15 @@ def __init__(self, client):
     async def create(self, params: CreateExtensionParams) -> ExtensionResponse:
         file_path, payload = normalize_extension_create_input(params)
 
-        try:
-            with open(file_path, "rb") as extension_file:
-                response = await self._client.transport.post(
-                    self._client._build_url("/extensions/add"),
-                    data=payload,
-                    files={"file": extension_file},
-                )
-        except OSError as exc:
-            raise HyperbrowserError(
-                f"Failed to open extension file at path: {file_path}",
-                original_error=exc,
-            ) from exc
+        with open_binary_file(
+            file_path,
+            open_error_message=f"Failed to open extension file at path: {file_path}",
+        ) as extension_file:
+            response = await self._client.transport.post(
+                self._client._build_url("/extensions/add"),
+                data=payload,
+                files={"file": extension_file},
+            )
         return parse_response_model(
             response.data,
             model=ExtensionResponse,

hyperbrowser/client/managers/async_manager/session.py

@@ -2,6 +2,7 @@
 from typing import IO, List, Optional, Union, overload
 import warnings
 from hyperbrowser.exceptions import HyperbrowserError
+from ...file_utils import open_binary_file
 from ..serialization_utils import (
     serialize_model_dump_or_default,
     serialize_model_dump_to_dict,
@@ -169,18 +170,15 @@ async def upload_file(
     ) -> UploadFileResponse:
         file_path, file_obj = normalize_upload_file_input(file_input)
         if file_path is not None:
-            try:
-                with open(file_path, "rb") as file_obj:
-                    files = {"file": file_obj}
-                    response = await self._client.transport.post(
-                        self._client._build_url(f"/session/{id}/uploads"),
-                        files=files,
-                    )
-            except OSError as exc:
-                raise HyperbrowserError(
-                    f"Failed to open upload file at path: {file_path}",
-                    original_error=exc,
-                ) from exc
+            with open_binary_file(
+                file_path,
+                open_error_message=f"Failed to open upload file at path: {file_path}",
+            ) as file_obj:
+                files = {"file": file_obj}
+                response = await self._client.transport.post(
+                    self._client._build_url(f"/session/{id}/uploads"),
+                    files=files,
+                )
         else:
             if file_obj is None:
                 raise HyperbrowserError(

hyperbrowser/client/managers/sync_manager/extension.py

@@ -1,6 +1,6 @@
 from typing import List
 
-from hyperbrowser.exceptions import HyperbrowserError
+from ...file_utils import open_binary_file
 from ..extension_create_utils import normalize_extension_create_input
 from ..extension_utils import parse_extension_list_response_data
 from ..response_utils import parse_response_model
@@ -14,18 +14,15 @@ def __init__(self, client):
     def create(self, params: CreateExtensionParams) -> ExtensionResponse:
         file_path, payload = normalize_extension_create_input(params)
 
-        try:
-            with open(file_path, "rb") as extension_file:
-                response = self._client.transport.post(
-                    self._client._build_url("/extensions/add"),
-                    data=payload,
-                    files={"file": extension_file},
-                )
-        except OSError as exc:
-            raise HyperbrowserError(
-                f"Failed to open extension file at path: {file_path}",
-                original_error=exc,
-            ) from exc
+        with open_binary_file(
+            file_path,
+            open_error_message=f"Failed to open extension file at path: {file_path}",
+        ) as extension_file:
+            response = self._client.transport.post(
+                self._client._build_url("/extensions/add"),
+                data=payload,
+                files={"file": extension_file},
+            )
         return parse_response_model(
             response.data,
             model=ExtensionResponse,

hyperbrowser/client/managers/sync_manager/session.py

@@ -2,6 +2,7 @@
 from typing import IO, List, Optional, Union, overload
 import warnings
 from hyperbrowser.exceptions import HyperbrowserError
+from ...file_utils import open_binary_file
 from ..serialization_utils import (
     serialize_model_dump_or_default,
     serialize_model_dump_to_dict,
@@ -161,18 +162,15 @@ def upload_file(
     ) -> UploadFileResponse:
         file_path, file_obj = normalize_upload_file_input(file_input)
         if file_path is not None:
-            try:
-                with open(file_path, "rb") as file_obj:
-                    files = {"file": file_obj}
-                    response = self._client.transport.post(
-                        self._client._build_url(f"/session/{id}/uploads"),
-                        files=files,
-                    )
-            except OSError as exc:
-                raise HyperbrowserError(
-                    f"Failed to open upload file at path: {file_path}",
-                    original_error=exc,
-                ) from exc
+            with open_binary_file(
+                file_path,
+                open_error_message=f"Failed to open upload file at path: {file_path}",
+            ) as file_obj:
+                files = {"file": file_obj}
+                response = self._client.transport.post(
+                    self._client._build_url(f"/session/{id}/uploads"),
+                    files=files,
+                )
         else:
             if file_obj is None:
                 raise HyperbrowserError(

tests/test_architecture_marker_usage.py

@@ -12,6 +12,7 @@
     "tests/test_mapping_keys_access_usage.py",
     "tests/test_tool_mapping_reader_usage.py",
     "tests/test_display_helper_usage.py",
+    "tests/test_binary_file_open_helper_usage.py",
     "tests/test_ci_workflow_quality_gates.py",
     "tests/test_makefile_quality_targets.py",
     "tests/test_pyproject_architecture_marker.py",

tests/test_binary_file_open_helper_usage.py

@@ -0,0 +1,20 @@
+from pathlib import Path
+
+import pytest
+
+pytestmark = pytest.mark.architecture
+
+
+MODULES = (
+    "hyperbrowser/client/managers/sync_manager/extension.py",
+    "hyperbrowser/client/managers/async_manager/extension.py",
+    "hyperbrowser/client/managers/sync_manager/session.py",
+    "hyperbrowser/client/managers/async_manager/session.py",
+)
+
+
+def test_managers_use_shared_binary_file_open_helper():
+    for module_path in MODULES:
+        module_text = Path(module_path).read_text(encoding="utf-8")
+        assert "open_binary_file(" in module_text
+        assert 'with open(file_path, "rb")' not in module_text

tests/test_file_utils.py

@@ -3,7 +3,7 @@
 import pytest
 
 import hyperbrowser.client.file_utils as file_utils
-from hyperbrowser.client.file_utils import ensure_existing_file_path
+from hyperbrowser.client.file_utils import ensure_existing_file_path, open_binary_file
 from hyperbrowser.exceptions import HyperbrowserError
 
 
@@ -480,3 +480,62 @@ def __iter__(self):
         )
 
     assert exc_info.value.original_error is None
+
+
+def test_open_binary_file_reads_content_and_closes(tmp_path: Path):
+    file_path = tmp_path / "binary.bin"
+    file_path.write_bytes(b"content")
+
+    with open_binary_file(
+        str(file_path),
+        open_error_message="open failed",
+    ) as file_obj:
+        assert file_obj.read() == b"content"
+        assert file_obj.closed is False
+
+    assert file_obj.closed is True
+
+
+def test_open_binary_file_rejects_non_string_error_message(tmp_path: Path):
+    file_path = tmp_path / "binary.bin"
+    file_path.write_bytes(b"content")
+
+    with pytest.raises(HyperbrowserError, match="open_error_message must be a string"):
+        with open_binary_file(
+            str(file_path),
+            open_error_message=123,  # type: ignore[arg-type]
+        ):
+            pass
+
+
+def test_open_binary_file_rejects_invalid_file_path_type():
+    with pytest.raises(
+        HyperbrowserError, match="file_path must be a string or os.PathLike object"
+    ):
+        with open_binary_file(
+            123,  # type: ignore[arg-type]
+            open_error_message="open failed",
+        ):
+            pass
+
+
+def test_open_binary_file_rejects_non_string_fspath_results():
+    with pytest.raises(HyperbrowserError, match="file_path must resolve to a string"):
+        with open_binary_file(
+            b"/tmp/bytes-path",  # type: ignore[arg-type]
+            open_error_message="open failed",
+        ):
+            pass
+
+
+def test_open_binary_file_wraps_open_errors(tmp_path: Path):
+    missing_path = tmp_path / "missing.bin"
+
+    with pytest.raises(HyperbrowserError, match="open failed") as exc_info:
+        with open_binary_file(
+            str(missing_path),
+            open_error_message="open failed",
+        ):
+            pass
+
+    assert isinstance(exc_info.value.original_error, FileNotFoundError)