fix: raise Error instead of assert for user input validation (#3245)

Author: pavelfeldman

playwright/_impl/_assertions.py

@@ -1103,7 +1103,8 @@ def to_expected_text_values(
     ignoreCase: Optional[bool] = None,
 ) -> Sequence[ExpectedTextValue]:
     out: List[ExpectedTextValue] = []
-    assert isinstance(items, (list, tuple))
+    if not isinstance(items, (list, tuple)):
+        raise Error("Expected a list or tuple of strings or patterns")
     for item in items:
         if isinstance(item, str):
             o = ExpectedTextValue(

playwright/_impl/_fetch.py

@@ -354,9 +354,8 @@ async def fetch(
             if isinstance(to_impl(urlOrRequest), network.Request)
             else None
         )
-        assert request or isinstance(
-            urlOrRequest, str
-        ), "First argument must be either URL string or Request"
+        if not request and not isinstance(urlOrRequest, str):
+            raise Error("First argument must be either URL string or Request")
         return await self._inner_fetch(
             request,
             url,
@@ -391,17 +390,16 @@ async def _inner_fetch(
     ) -> "APIResponse":
         if self._close_reason:
             raise TargetClosedError(self._close_reason)
-        assert (
+        if (
             (1 if data is not None else 0)
             + (1 if form is not None else 0)
             + (1 if multipart is not None else 0)
-        ) <= 1, "Only one of 'data', 'form' or 'multipart' can be specified"
-        assert (
-            maxRedirects is None or maxRedirects >= 0
-        ), "'max_redirects' must be greater than or equal to '0'"
-        assert (
-            maxRetries is None or maxRetries >= 0
-        ), "'max_retries' must be greater than or equal to '0'"
+        ) > 1:
+            raise Error("Only one of 'data', 'form' or 'multipart' can be specified")
+        if maxRedirects is not None and maxRedirects < 0:
+            raise Error("'max_redirects' must be greater than or equal to '0'")
+        if maxRetries is not None and maxRetries < 0:
+            raise Error("'max_retries' must be greater than or equal to '0'")
         url = url or (request.url if request else url)
         method = method or (request.method if request else "GET")
         # Cannot call allHeaders() here as the request may be paused inside route handler.
@@ -518,9 +516,8 @@ async def _form_data_field_to_form_field(name: str, value: Any) -> FormField:
         )
     if is_file_payload(value):
         payload = cast(FilePayload, value)
-        assert isinstance(
-            payload["buffer"], bytes
-        ), f"Unexpected buffer type of form field {name!r}"
+        if not isinstance(payload["buffer"], bytes):
+            raise Error(f"Unexpected buffer type of form field {name!r}")
         return FormField(name=name, file=file_payload_to_json(payload))
     if isinstance(value, (str, int, float, bool)):
         return FormField(name=name, value=str(value))

playwright/_impl/_js_handle.py

@@ -185,7 +185,8 @@ def serialize_value(
     if isinstance(value, ParseResult):
         return {"u": urlunparse(value)}
     if isinstance(value, Pattern):
-        assert isinstance(value.pattern, str), "Only str patterns are supported."
+        if not isinstance(value.pattern, str):
+            raise Error("Only str patterns are supported.")
         return {"r": {"p": value.pattern, "f": escape_regex_flags(value)}}
 
     if value in visitor_info.visited:

playwright/_impl/_str_utils.py

@@ -16,6 +16,8 @@
 import re
 from typing import Pattern, Union
 
+from playwright._impl._errors import Error
+
 
 def escape_regex_flags(pattern: Pattern) -> str:
     flags = ""
@@ -27,11 +29,14 @@ def escape_regex_flags(pattern: Pattern) -> str:
         flags += "s"
     if (pattern.flags & int(re.MULTILINE)) != 0:
         flags += "m"
-    assert (
+    if (
         pattern.flags
         & ~(int(re.MULTILINE) | int(re.IGNORECASE) | int(re.DOTALL) | int(re.UNICODE))
-        == 0
-    ), "Unexpected re.Pattern flag, only MULTILINE, IGNORECASE and DOTALL are supported."
+        != 0
+    ):
+        raise Error(
+            "Unexpected re.Pattern flag, only MULTILINE, IGNORECASE and DOTALL are supported."
+        )
     return flags
 
 

tests/async/test_fetch_global.py

@@ -467,7 +467,7 @@ async def test_should_throw_an_error_when_max_redirects_is_less_than_0(
 ) -> None:
     request = await playwright.request.new_context()
     for method in ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]:
-        with pytest.raises(AssertionError) as exc_info:
+        with pytest.raises(Error) as exc_info:
             await request.fetch(
                 server.PREFIX + "/a/redirect1", method=method, max_redirects=-1
             )
@@ -507,7 +507,7 @@ async def test_should_reject_empty_data_with_form(
 ) -> None:
     request = await playwright.request.new_context()
     with pytest.raises(
-        AssertionError,
+        Error,
         match="Only one of 'data', 'form' or 'multipart' can be specified",
     ):
         await request.post(

tests/async/test_page_evaluate.py

@@ -397,7 +397,7 @@ async def test_evaluate_return_regex_unsupported_by_python(page: Page) -> None:
 
 
 async def test_evaluate_throw_for_unsupported_regex(page: Page) -> None:
-    with pytest.raises(AssertionError, match="Unexpected re.Pattern flag"):
+    with pytest.raises(Error, match="Unexpected re.Pattern flag"):
         await page.evaluate("a => a", re.compile("foo", re.VERBOSE))
-    with pytest.raises(AssertionError, match="Only str patterns are supported"):
+    with pytest.raises(Error, match="Only str patterns are supported"):
         await page.evaluate("a => a", re.compile(b"foo"))

tests/sync/test_fetch_global.py

@@ -318,7 +318,7 @@ def test_should_throw_an_error_when_max_redirects_is_less_than_0(
 ) -> None:
     request = playwright.request.new_context()
     for method in ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]:
-        with pytest.raises(AssertionError) as exc_info:
+        with pytest.raises(Error) as exc_info:
             request.fetch(
                 server.PREFIX + "/a/redirect1", method=method, max_redirects=-1
             )
@@ -351,7 +351,7 @@ def test_should_reject_empty_data_with_form(
 ) -> None:
     request = playwright.request.new_context()
     with pytest.raises(
-        AssertionError,
+        Error,
         match="Only one of 'data', 'form' or 'multipart' can be specified",
     ):
         request.post(