Harden open-file helper default-prefix fallback behavior

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

Author: cursoragent

hyperbrowser/client/file_utils.py

@@ -90,11 +90,20 @@ def build_file_path_error_message(
     return f"{normalized_prefix}: {file_path_display}"
 
 
-def build_open_file_error_message(file_path: object, *, prefix: str) -> str:
+def build_open_file_error_message(
+    file_path: object,
+    *,
+    prefix: str,
+    default_prefix: Optional[str] = None,
+) -> str:
     return build_file_path_error_message(
         file_path,
         prefix=prefix,
-        default_prefix=_DEFAULT_OPEN_ERROR_MESSAGE_PREFIX,
+        default_prefix=(
+            _DEFAULT_OPEN_ERROR_MESSAGE_PREFIX
+            if default_prefix is None
+            else default_prefix
+        ),
     )
 
 

hyperbrowser/client/managers/async_manager/extension.py

@@ -30,6 +30,7 @@ async def create(self, params: CreateExtensionParams) -> ExtensionResponse:
             open_error_message=build_open_file_error_message(
                 file_path,
                 prefix=self._OPERATION_METADATA.open_file_error_prefix,
+                default_prefix="Failed to open extension file at path",
             ),
         ) as extension_file:
             return await create_extension_resource_async(

hyperbrowser/client/managers/session_upload_utils.py

@@ -84,6 +84,7 @@ def open_upload_files_from_input(
             open_error_message=build_open_file_error_message(
                 file_path,
                 prefix=SESSION_OPERATION_METADATA.upload_open_file_error_prefix,
+                default_prefix="Failed to open upload file at path",
             ),
         ) as opened_file:
             yield {"file": opened_file}

hyperbrowser/client/managers/sync_manager/extension.py

@@ -30,6 +30,7 @@ def create(self, params: CreateExtensionParams) -> ExtensionResponse:
             open_error_message=build_open_file_error_message(
                 file_path,
                 prefix=self._OPERATION_METADATA.open_file_error_prefix,
+                default_prefix="Failed to open extension file at path",
             ),
         ) as extension_file:
             return create_extension_resource(

tests/test_extension_manager.py

@@ -207,6 +207,53 @@ def _open_binary_file_stub(file_path, *, open_error_message):  # type: ignore[no
     assert captured["open_error_message"] == "Custom extension open prefix: bad?path.zip"
 
 
+def test_sync_extension_create_uses_default_open_file_prefix_when_metadata_invalid(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = SyncExtensionManager(_FakeClient(_SyncTransport()))
+    manager._OPERATION_METADATA = type(
+        "_Metadata",
+        (),
+        {
+            "create_operation_name": "create extension",
+            "open_file_error_prefix": 123,
+        },
+    )()
+    params = CreateExtensionParams(name="my-extension", file_path="/tmp/ignored.zip")
+    captured: dict[str, str] = {}
+
+    @contextmanager
+    def _open_binary_file_stub(file_path, *, open_error_message):  # type: ignore[no-untyped-def]
+        captured["file_path"] = file_path
+        captured["open_error_message"] = open_error_message
+        yield io.BytesIO(b"content")
+
+    monkeypatch.setattr(
+        sync_extension_module,
+        "normalize_extension_create_input",
+        lambda _: ("bad\tpath.zip", {"name": "my-extension"}),
+    )
+    monkeypatch.setattr(
+        sync_extension_module,
+        "open_binary_file",
+        _open_binary_file_stub,
+    )
+    monkeypatch.setattr(
+        sync_extension_module,
+        "create_extension_resource",
+        lambda **kwargs: SimpleNamespace(id="ext_sync_mock"),
+    )
+
+    response = manager.create(params)
+
+    assert response.id == "ext_sync_mock"
+    assert captured["file_path"] == "bad\tpath.zip"
+    assert (
+        captured["open_error_message"]
+        == "Failed to open extension file at path: bad?path.zip"
+    )
+
+
 def test_async_extension_create_does_not_mutate_params_and_closes_file(tmp_path):
     transport = _AsyncTransport()
     manager = AsyncExtensionManager(_FakeClient(transport))
@@ -323,6 +370,60 @@ async def run():
     assert captured["open_error_message"] == "Custom extension open prefix: bad?path.zip"
 
 
+def test_async_extension_create_uses_default_open_file_prefix_when_metadata_invalid(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    manager = AsyncExtensionManager(_FakeClient(_AsyncTransport()))
+    manager._OPERATION_METADATA = type(
+        "_Metadata",
+        (),
+        {
+            "create_operation_name": "create extension",
+            "open_file_error_prefix": 123,
+        },
+    )()
+    params = CreateExtensionParams(name="my-extension", file_path="/tmp/ignored.zip")
+    captured: dict[str, str] = {}
+
+    @contextmanager
+    def _open_binary_file_stub(file_path, *, open_error_message):  # type: ignore[no-untyped-def]
+        captured["file_path"] = file_path
+        captured["open_error_message"] = open_error_message
+        yield io.BytesIO(b"content")
+
+    async def _create_extension_resource_async_stub(**kwargs):
+        _ = kwargs
+        return SimpleNamespace(id="ext_async_mock")
+
+    monkeypatch.setattr(
+        async_extension_module,
+        "normalize_extension_create_input",
+        lambda _: ("bad\tpath.zip", {"name": "my-extension"}),
+    )
+    monkeypatch.setattr(
+        async_extension_module,
+        "open_binary_file",
+        _open_binary_file_stub,
+    )
+    monkeypatch.setattr(
+        async_extension_module,
+        "create_extension_resource_async",
+        _create_extension_resource_async_stub,
+    )
+
+    async def run():
+        return await manager.create(params)
+
+    response = asyncio.run(run())
+
+    assert response.id == "ext_async_mock"
+    assert captured["file_path"] == "bad\tpath.zip"
+    assert (
+        captured["open_error_message"]
+        == "Failed to open extension file at path: bad?path.zip"
+    )
+
+
 def test_sync_extension_create_raises_hyperbrowser_error_when_file_missing(tmp_path):
     transport = _SyncTransport()
     manager = SyncExtensionManager(_FakeClient(transport))

tests/test_extension_operation_metadata_usage.py

@@ -1,3 +1,4 @@
+import re
 from pathlib import Path
 
 import pytest
@@ -19,4 +20,10 @@ def test_extension_managers_use_shared_operation_metadata():
         assert "operation_name=self._OPERATION_METADATA." in module_text
         assert "prefix=self._OPERATION_METADATA.open_file_error_prefix" in module_text
         assert 'operation_name="' not in module_text
-        assert 'prefix="Failed to open extension file at path"' not in module_text
+        assert (
+            re.search(
+                r'(?<!default_)prefix="Failed to open extension file at path"',
+                module_text,
+            )
+            is None
+        )

tests/test_file_open_error_helper_usage.py

@@ -16,4 +16,5 @@ def test_file_open_error_messages_use_shared_helper():
     for module_path in OPEN_ERROR_HELPER_MODULES:
         module_text = Path(module_path).read_text(encoding="utf-8")
         assert "build_open_file_error_message(" in module_text
+        assert "default_prefix=" in module_text
         assert "open_error_message=f\"" not in module_text

tests/test_file_utils.py

@@ -399,6 +399,16 @@ def test_build_open_file_error_message_uses_default_prefix_for_blank_string():
     assert message == "Failed to open file at path: /tmp/path.txt"
 
 
+def test_build_open_file_error_message_uses_explicit_default_prefix_when_prefix_invalid():
+    message = build_open_file_error_message(
+        "/tmp/path.txt",
+        prefix=123,  # type: ignore[arg-type]
+        default_prefix="Failed to open upload file at path",
+    )
+
+    assert message == "Failed to open upload file at path: /tmp/path.txt"
+
+
 def test_build_open_file_error_message_sanitizes_control_chars_in_prefix():
     message = build_open_file_error_message(
         "/tmp/path.txt",

tests/test_session_upload_utils.py

@@ -304,6 +304,48 @@ def _open_binary_file_stub(file_path, *, open_error_message):  # type: ignore[no
     assert captured["open_error_message"] == "Custom open prefix: bad?path.txt"
 
 
+def test_open_upload_files_from_input_uses_default_open_prefix_when_metadata_invalid(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    captured: dict[str, str] = {}
+
+    @contextmanager
+    def _open_binary_file_stub(file_path, *, open_error_message):  # type: ignore[no-untyped-def]
+        captured["file_path"] = file_path
+        captured["open_error_message"] = open_error_message
+        yield io.BytesIO(b"content")
+
+    monkeypatch.setattr(
+        session_upload_utils,
+        "normalize_upload_file_input",
+        lambda file_input: ("bad\tpath.txt", None),
+    )
+    monkeypatch.setattr(
+        session_upload_utils,
+        "SESSION_OPERATION_METADATA",
+        type(
+            "_Metadata",
+            (),
+            {
+                "upload_missing_file_message_prefix": "Custom missing prefix",
+                "upload_not_file_message_prefix": "Custom not-file prefix",
+                "upload_open_file_error_prefix": 123,
+            },
+        )(),
+    )
+    monkeypatch.setattr(
+        session_upload_utils,
+        "open_binary_file",
+        _open_binary_file_stub,
+    )
+
+    with open_upload_files_from_input("ignored-input") as files:
+        assert files["file"].read() == b"content"
+
+    assert captured["file_path"] == "bad\tpath.txt"
+    assert captured["open_error_message"] == "Failed to open upload file at path: bad?path.txt"
+
+
 def test_open_upload_files_from_input_rejects_missing_normalized_file_object(
     monkeypatch: pytest.MonkeyPatch,
 ):