chore: fix the review findings on the typed channels

playwright/_impl/_cdp_session.py

@@ -39,7 +39,10 @@ def __init__(
 
     def _on_event(self, event: "channels.CDPSessionEventEvent") -> None:
         self.emit(event["method"], event["params"])
-        self.emit(CDPSession.Events.Event, event)
+        self.emit(
+            CDPSession.Events.Event,
+            {key: value for key, value in event.items() if value is not None},
+        )
 
     async def send(self, method: str, params: Optional[Dict]) -> Dict:
         return (await self._channel.send(method=method, params=params))["result"]

playwright/_impl/_channel.py

@@ -0,0 +1,129 @@
+# Copyright (c) Microsoft Corporation.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Port of the channel proxy in packages/playwright-core/src/client/channelOwner.ts."""
+
+import asyncio
+from typing import TYPE_CHECKING, Any, Callable, Dict, List, Mapping, Optional, Tuple
+
+from pyee.asyncio import AsyncIOEventEmitter
+
+if TYPE_CHECKING:  # pragma: no cover
+    from playwright._impl._connection import ChannelOwner, Connection
+
+
+class Channel:
+    """The base of the generated channels in _protocol_channels.py."""
+
+    def __init__(self, connection: "Connection", object: "ChannelOwner") -> None:
+        self._connection = connection
+        self._guid = object._guid
+        self._object = object
+        self._emitter = AsyncIOEventEmitter()
+        self._emitter.on(
+            "error", lambda exc: self._connection._on_event_listener_error(exc)
+        )
+        # (event, handler) -> the wrappers registered for it, in order.
+        self._event_listeners: Dict[Tuple[str, Any], List[Callable[[Any], Any]]] = {}
+
+    async def _call(
+        self, method: str, params: Mapping[str, Any], timeout: float
+    ) -> Dict[str, Any]:
+        """Sends a command and returns its result, {} when it has none."""
+        result = await self._connection.wrap_api_call(
+            lambda: self._inner_send(method, _filter_none(params), timeout)
+        )
+        return result or {}
+
+    def _add_event_listener(
+        self, event: str, handler: Any, listener: Callable[[Any], Any]
+    ) -> None:
+        """Registers listener, the wrapper that calls handler with the typed payload.
+
+        A handler can be registered more than once, like on an EventEmitter;
+        each off<Event> call removes one registration.
+        """
+        self._event_listeners.setdefault((event, handler), []).append(listener)
+        self._emitter.on(event, listener)
+
+    def _remove_event_listener(self, event: str, handler: Any) -> None:
+        listeners = self._event_listeners.get((event, handler))
+        if not listeners:
+            return
+        listener = listeners.pop()
+        if not listeners:
+            del self._event_listeners[(event, handler)]
+        self._emitter.remove_listener(event, listener)
+
+    def send_no_reply(
+        self,
+        method: str,
+        params: Mapping[str, Any] = None,
+        is_internal: bool = False,
+        title: str = None,
+    ) -> None:
+        """Sends a message without waiting for the reply.
+
+        For messages that must go out synchronously, such as __waitInfo__ and
+        updateSubscription.
+        """
+        self._connection.wrap_api_call_sync(
+            lambda: self._connection._send_message_to_server(
+                self._object, method, _filter_none(params or {}), 0, True
+            ),
+            is_internal,
+            title,
+        )
+
+    async def _inner_send(
+        self, method: str, params: Dict[str, Any], timeout: float
+    ) -> Optional[Dict[str, Any]]:
+        if self._connection._error:
+            error = self._connection._error
+            self._connection._error = None
+            raise error
+        callback = self._connection._send_message_to_server(
+            self._object, method, params, timeout
+        )
+        try:
+            done, _ = await asyncio.wait(
+                {
+                    self._connection._transport.on_error_future,
+                    callback.future,
+                },
+                return_when=asyncio.FIRST_COMPLETED,
+            )
+        except asyncio.CancelledError as exc:
+            await self._connection._abort(
+                self._object,
+                callback,
+                str(exc) or "Task was cancelled",
+            )
+            raise
+        if not callback.future.done():
+            callback.future.cancel()
+        result = next(iter(done)).result()
+        if not result:
+            return None
+        assert isinstance(result, dict)
+        return result
+
+
+def _filter_none(d: Mapping) -> Dict:
+    result = {}
+    for k, v in d.items():
+        if v is None:
+            continue
+        result[k] = _filter_none(v) if isinstance(v, dict) else v
+    return result

playwright/_impl/_connection.py

@@ -27,9 +27,7 @@
     Callable,
     Dict,
     List,
-    Mapping,
     Optional,
-    Tuple,
     TypedDict,
     Union,
     cast,
@@ -40,6 +38,7 @@
 
 import playwright
 import playwright._impl._impl_to_api_mapping
+from playwright._impl._channel import Channel
 from playwright._impl._errors import (
     Error,
     TargetClosedError,
@@ -50,6 +49,7 @@
 from playwright._impl._helper import (
     ParsedMessagePayload,
 )
+from playwright._impl._protocol_channels import CHANNEL_CLASSES
 from playwright._impl._protocol_check import check_message, is_enabled
 from playwright._impl._transport import Transport
 
@@ -61,96 +61,6 @@
 _PLAYWRIGHT_MODULE_PATH = str(Path(playwright.__file__).parents[0])
 
 
-class Channel:
-    """The base of the generated channels in _protocol_channels.py.
-
-    Port of the channel proxy in packages/playwright-core/src/client/channelOwner.ts.
-    """
-
-    def __init__(self, connection: "Connection", object: "ChannelOwner") -> None:
-        self._connection = connection
-        self._guid = object._guid
-        self._object = object
-        self._emitter = AsyncIOEventEmitter()
-        self._emitter.on(
-            "error", lambda exc: self._connection._on_event_listener_error(exc)
-        )
-        self._event_listeners: Dict[Tuple[str, Any], Callable[[Any], Any]] = {}
-
-    async def _call(
-        self, method: str, params: Mapping[str, Any], timeout: float
-    ) -> Dict[str, Any]:
-        """Sends a command and returns its result, {} when it has none."""
-        result = await self._connection.wrap_api_call(
-            lambda: self._inner_send(method, _filter_none(params), timeout)
-        )
-        return result or {}
-
-    def _add_event_listener(
-        self, event: str, handler: Any, listener: Callable[[Any], Any]
-    ) -> None:
-        self._event_listeners[(event, handler)] = listener
-        self._emitter.on(event, listener)
-
-    def _remove_event_listener(self, event: str, handler: Any) -> None:
-        listener = self._event_listeners.pop((event, handler), None)
-        if listener:
-            self._emitter.remove_listener(event, listener)
-
-    def send_no_reply(
-        self,
-        method: str,
-        params: Mapping[str, Any] = None,
-        is_internal: bool = False,
-        title: str = None,
-    ) -> None:
-        """Sends a message without waiting for the reply.
-
-        For messages that must go out synchronously, such as __waitInfo__ and
-        updateSubscription.
-        """
-        self._connection.wrap_api_call_sync(
-            lambda: self._connection._send_message_to_server(
-                self._object, method, _filter_none(params or {}), 0, True
-            ),
-            is_internal,
-            title,
-        )
-
-    async def _inner_send(
-        self, method: str, params: Dict[str, Any], timeout: float
-    ) -> Optional[Dict[str, Any]]:
-        if self._connection._error:
-            error = self._connection._error
-            self._connection._error = None
-            raise error
-        callback = self._connection._send_message_to_server(
-            self._object, method, params, timeout
-        )
-        try:
-            done, _ = await asyncio.wait(
-                {
-                    self._connection._transport.on_error_future,
-                    callback.future,
-                },
-                return_when=asyncio.FIRST_COMPLETED,
-            )
-        except asyncio.CancelledError as exc:
-            await self._connection._abort(
-                self._object,
-                callback,
-                str(exc) or "Task was cancelled",
-            )
-            raise
-        if not callback.future.done():
-            callback.future.cancel()
-        result = next(iter(done)).result()
-        if not result:
-            return None
-        assert isinstance(result, dict)
-        return result
-
-
 class ChannelOwner(AsyncIOEventEmitter):
     def __init__(
         self,
@@ -171,9 +81,6 @@ def __init__(
             parent if isinstance(parent, ChannelOwner) else None
         )
         self._objects: Dict[str, "ChannelOwner"] = {}
-        # Imported here: the generated channels import this module.
-        from playwright._impl._protocol_channels import CHANNEL_CLASSES
-
         self._channel: Channel = CHANNEL_CLASSES.get(type, Channel)(
             self._connection, self
         )
@@ -722,15 +629,6 @@ def _attach_api_call_information(
     return stack_trace_information
 
 
-def _filter_none(d: Mapping) -> Dict:
-    result = {}
-    for k, v in d.items():
-        if v is None:
-            continue
-        result[k] = _filter_none(v) if isinstance(v, dict) else v
-    return result
-
-
 def format_call_log(log: Optional[List[str]]) -> str:
     if not log:
         return ""

playwright/_impl/_js_handle.py

@@ -17,7 +17,8 @@
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
 
-from playwright._impl._connection import Channel, ChannelOwner, from_channel
+from playwright._impl._channel import Channel
+from playwright._impl._connection import ChannelOwner, from_channel
 from playwright._impl._errors import is_target_closed_error
 from playwright._impl._serializers import parse_value, serialize_value
 

playwright/_impl/_network.py

@@ -445,6 +445,8 @@ async def _inner_fulfill(
             encoded_body = base64.b64encode(body).decode()
             is_base64 = True
             length = len(body)
+        elif body is not None:
+            raise Error(f"body must be a str or bytes, got {type(body).__name__}")
         elif path:
             file_content = Path(path).read_bytes()
             encoded_body = base64.b64encode(file_content).decode()

playwright/_impl/_protocol_channels.py

@@ -28,7 +28,7 @@
 
 from typing import TYPE_CHECKING, Any, Callable, Dict, Literal, Optional, Sequence, Type
 
-from playwright._impl._connection import Channel
+from playwright._impl._channel import Channel
 
 if TYPE_CHECKING:
     from playwright._impl import _protocol_types as channels

playwright/_impl/_protocol_types.py

@@ -45,7 +45,7 @@
 if TYPE_CHECKING:
     from typing_extensions import NotRequired
 
-    from playwright._impl._connection import Channel
+    from playwright._impl._channel import Channel
     from playwright._impl._protocol_channels import (
         AndroidChannel,
         AndroidDeviceChannel,

playwright/_impl/_set_input_files_helpers.py

@@ -83,25 +83,30 @@ async def convert_input_files(
                 (_list_files(local_directory) if local_directory else local_paths),
             )
             streams = []
-            root_dir_name = (
-                os.path.basename(local_directory) if local_directory else None
-            )
-            temp_files: List[channels.BrowserContextCreateTempFilesParamsItems] = [
-                {
-                    "name": (
-                        os.path.relpath(file, local_directory)
-                        if local_directory
-                        else os.path.basename(file)
+
+            async def create_temp_files() -> (
+                "channels.BrowserContextCreateTempFilesResult"
+            ):
+                return await context._channel.createTempFiles(
+                    rootDirName=(
+                        os.path.basename(local_directory) if local_directory else None
                     ),
-                    "lastModifiedMs": int(os.path.getmtime(file) * 1000),
-                }
-                for file in files_to_stream
-            ]
+                    items=[
+                        {
+                            "name": (
+                                os.path.relpath(file, local_directory)
+                                if local_directory
+                                else os.path.basename(file)
+                            ),
+                            "lastModifiedMs": int(os.path.getmtime(file) * 1000),
+                        }
+                        for file in files_to_stream
+                    ],
+                )
+
             result: channels.BrowserContextCreateTempFilesResult = (
                 await context._connection.wrap_api_call(
-                    lambda: context._channel.createTempFiles(
-                        rootDirName=root_dir_name, items=temp_files
-                    )
+                    create_temp_files, is_internal=True
                 )
             )
             for i, file in enumerate(result["writableStreams"]):

scripts/codegen/generate_protocol.py

@@ -358,7 +358,7 @@ def generate_types(protocol: Dict[str, Any]) -> Tuple[str, List[Interface], Set[
             "if TYPE_CHECKING:",
             "    from typing_extensions import NotRequired",
             "",
-            "    from playwright._impl._connection import Channel",
+            "    from playwright._impl._channel import Channel",
             "    from playwright._impl._protocol_channels import (",
             *[f"        {channel}," for channel in used_channels],
             "    )",
@@ -461,7 +461,7 @@ def generate_channels(interfaces: List[Interface], type_names: Set[str]) -> str:
             "",
             f"from typing import {', '.join(sorted(typing_names))}",
             "",
-            "from playwright._impl._connection import Channel",
+            "from playwright._impl._channel import Channel",
             "",
             "if TYPE_CHECKING:",
             "    from playwright._impl import _protocol_types as channels",