Skip to content

Commit 3e7abc4

Browse files
authored
chore: fix the review findings on the typed channels (#3268)
1 parent 9d198c0 commit 3e7abc4

9 files changed

Lines changed: 164 additions & 126 deletions

File tree

‎playwright/_impl/_cdp_session.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ def __init__(
3939

4040
def _on_event(self, event: "channels.CDPSessionEventEvent") -> None:
4141
self.emit(event["method"], event["params"])
42-
self.emit(CDPSession.Events.Event, event)
42+
self.emit(
43+
CDPSession.Events.Event,
44+
{key: value for key, value in event.items() if value is not None},
45+
)
4346

4447
async def send(self, method: str, params: Optional[Dict]) -> Dict:
4548
return (await self._channel.send(method=method, params=params))["result"]

‎playwright/_impl/_channel.py‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
# Copyright (c) Microsoft Corporation.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Port of the channel proxy in packages/playwright-core/src/client/channelOwner.ts."""
16+
17+
import asyncio
18+
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Mapping, Optional, Tuple
19+
20+
from pyee.asyncio import AsyncIOEventEmitter
21+
22+
if TYPE_CHECKING: # pragma: no cover
23+
from playwright._impl._connection import ChannelOwner, Connection
24+
25+
26+
class Channel:
27+
"""The base of the generated channels in _protocol_channels.py."""
28+
29+
def __init__(self, connection: "Connection", object: "ChannelOwner") -> None:
30+
self._connection = connection
31+
self._guid = object._guid
32+
self._object = object
33+
self._emitter = AsyncIOEventEmitter()
34+
self._emitter.on(
35+
"error", lambda exc: self._connection._on_event_listener_error(exc)
36+
)
37+
# (event, handler) -> the wrappers registered for it, in order.
38+
self._event_listeners: Dict[Tuple[str, Any], List[Callable[[Any], Any]]] = {}
39+
40+
async def _call(
41+
self, method: str, params: Mapping[str, Any], timeout: float
42+
) -> Dict[str, Any]:
43+
"""Sends a command and returns its result, {} when it has none."""
44+
result = await self._connection.wrap_api_call(
45+
lambda: self._inner_send(method, _filter_none(params), timeout)
46+
)
47+
return result or {}
48+
49+
def _add_event_listener(
50+
self, event: str, handler: Any, listener: Callable[[Any], Any]
51+
) -> None:
52+
"""Registers listener, the wrapper that calls handler with the typed payload.
53+
54+
A handler can be registered more than once, like on an EventEmitter;
55+
each off<Event> call removes one registration.
56+
"""
57+
self._event_listeners.setdefault((event, handler), []).append(listener)
58+
self._emitter.on(event, listener)
59+
60+
def _remove_event_listener(self, event: str, handler: Any) -> None:
61+
listeners = self._event_listeners.get((event, handler))
62+
if not listeners:
63+
return
64+
listener = listeners.pop()
65+
if not listeners:
66+
del self._event_listeners[(event, handler)]
67+
self._emitter.remove_listener(event, listener)
68+
69+
def send_no_reply(
70+
self,
71+
method: str,
72+
params: Mapping[str, Any] = None,
73+
is_internal: bool = False,
74+
title: str = None,
75+
) -> None:
76+
"""Sends a message without waiting for the reply.
77+
78+
For messages that must go out synchronously, such as __waitInfo__ and
79+
updateSubscription.
80+
"""
81+
self._connection.wrap_api_call_sync(
82+
lambda: self._connection._send_message_to_server(
83+
self._object, method, _filter_none(params or {}), 0, True
84+
),
85+
is_internal,
86+
title,
87+
)
88+
89+
async def _inner_send(
90+
self, method: str, params: Dict[str, Any], timeout: float
91+
) -> Optional[Dict[str, Any]]:
92+
if self._connection._error:
93+
error = self._connection._error
94+
self._connection._error = None
95+
raise error
96+
callback = self._connection._send_message_to_server(
97+
self._object, method, params, timeout
98+
)
99+
try:
100+
done, _ = await asyncio.wait(
101+
{
102+
self._connection._transport.on_error_future,
103+
callback.future,
104+
},
105+
return_when=asyncio.FIRST_COMPLETED,
106+
)
107+
except asyncio.CancelledError as exc:
108+
await self._connection._abort(
109+
self._object,
110+
callback,
111+
str(exc) or "Task was cancelled",
112+
)
113+
raise
114+
if not callback.future.done():
115+
callback.future.cancel()
116+
result = next(iter(done)).result()
117+
if not result:
118+
return None
119+
assert isinstance(result, dict)
120+
return result
121+
122+
123+
def _filter_none(d: Mapping) -> Dict:
124+
result = {}
125+
for k, v in d.items():
126+
if v is None:
127+
continue
128+
result[k] = _filter_none(v) if isinstance(v, dict) else v
129+
return result

‎playwright/_impl/_connection.py‎

Lines changed: 2 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@
2727
Callable,
2828
Dict,
2929
List,
30-
Mapping,
3130
Optional,
32-
Tuple,
3331
TypedDict,
3432
Union,
3533
cast,
@@ -40,6 +38,7 @@
4038

4139
import playwright
4240
import playwright._impl._impl_to_api_mapping
41+
from playwright._impl._channel import Channel
4342
from playwright._impl._errors import (
4443
Error,
4544
TargetClosedError,
@@ -50,6 +49,7 @@
5049
from playwright._impl._helper import (
5150
ParsedMessagePayload,
5251
)
52+
from playwright._impl._protocol_channels import CHANNEL_CLASSES
5353
from playwright._impl._protocol_check import check_message, is_enabled
5454
from playwright._impl._transport import Transport
5555

@@ -61,96 +61,6 @@
6161
_PLAYWRIGHT_MODULE_PATH = str(Path(playwright.__file__).parents[0])
6262

6363

64-
class Channel:
65-
"""The base of the generated channels in _protocol_channels.py.
66-
67-
Port of the channel proxy in packages/playwright-core/src/client/channelOwner.ts.
68-
"""
69-
70-
def __init__(self, connection: "Connection", object: "ChannelOwner") -> None:
71-
self._connection = connection
72-
self._guid = object._guid
73-
self._object = object
74-
self._emitter = AsyncIOEventEmitter()
75-
self._emitter.on(
76-
"error", lambda exc: self._connection._on_event_listener_error(exc)
77-
)
78-
self._event_listeners: Dict[Tuple[str, Any], Callable[[Any], Any]] = {}
79-
80-
async def _call(
81-
self, method: str, params: Mapping[str, Any], timeout: float
82-
) -> Dict[str, Any]:
83-
"""Sends a command and returns its result, {} when it has none."""
84-
result = await self._connection.wrap_api_call(
85-
lambda: self._inner_send(method, _filter_none(params), timeout)
86-
)
87-
return result or {}
88-
89-
def _add_event_listener(
90-
self, event: str, handler: Any, listener: Callable[[Any], Any]
91-
) -> None:
92-
self._event_listeners[(event, handler)] = listener
93-
self._emitter.on(event, listener)
94-
95-
def _remove_event_listener(self, event: str, handler: Any) -> None:
96-
listener = self._event_listeners.pop((event, handler), None)
97-
if listener:
98-
self._emitter.remove_listener(event, listener)
99-
100-
def send_no_reply(
101-
self,
102-
method: str,
103-
params: Mapping[str, Any] = None,
104-
is_internal: bool = False,
105-
title: str = None,
106-
) -> None:
107-
"""Sends a message without waiting for the reply.
108-
109-
For messages that must go out synchronously, such as __waitInfo__ and
110-
updateSubscription.
111-
"""
112-
self._connection.wrap_api_call_sync(
113-
lambda: self._connection._send_message_to_server(
114-
self._object, method, _filter_none(params or {}), 0, True
115-
),
116-
is_internal,
117-
title,
118-
)
119-
120-
async def _inner_send(
121-
self, method: str, params: Dict[str, Any], timeout: float
122-
) -> Optional[Dict[str, Any]]:
123-
if self._connection._error:
124-
error = self._connection._error
125-
self._connection._error = None
126-
raise error
127-
callback = self._connection._send_message_to_server(
128-
self._object, method, params, timeout
129-
)
130-
try:
131-
done, _ = await asyncio.wait(
132-
{
133-
self._connection._transport.on_error_future,
134-
callback.future,
135-
},
136-
return_when=asyncio.FIRST_COMPLETED,
137-
)
138-
except asyncio.CancelledError as exc:
139-
await self._connection._abort(
140-
self._object,
141-
callback,
142-
str(exc) or "Task was cancelled",
143-
)
144-
raise
145-
if not callback.future.done():
146-
callback.future.cancel()
147-
result = next(iter(done)).result()
148-
if not result:
149-
return None
150-
assert isinstance(result, dict)
151-
return result
152-
153-
15464
class ChannelOwner(AsyncIOEventEmitter):
15565
def __init__(
15666
self,
@@ -171,9 +81,6 @@ def __init__(
17181
parent if isinstance(parent, ChannelOwner) else None
17282
)
17383
self._objects: Dict[str, "ChannelOwner"] = {}
174-
# Imported here: the generated channels import this module.
175-
from playwright._impl._protocol_channels import CHANNEL_CLASSES
176-
17784
self._channel: Channel = CHANNEL_CLASSES.get(type, Channel)(
17885
self._connection, self
17986
)
@@ -722,15 +629,6 @@ def _attach_api_call_information(
722629
return stack_trace_information
723630

724631

725-
def _filter_none(d: Mapping) -> Dict:
726-
result = {}
727-
for k, v in d.items():
728-
if v is None:
729-
continue
730-
result[k] = _filter_none(v) if isinstance(v, dict) else v
731-
return result
732-
733-
734632
def format_call_log(log: Optional[List[str]]) -> str:
735633
if not log:
736634
return ""

‎playwright/_impl/_js_handle.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
from pathlib import Path
1818
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
1919

20-
from playwright._impl._connection import Channel, ChannelOwner, from_channel
20+
from playwright._impl._channel import Channel
21+
from playwright._impl._connection import ChannelOwner, from_channel
2122
from playwright._impl._errors import is_target_closed_error
2223
from playwright._impl._serializers import parse_value, serialize_value
2324

‎playwright/_impl/_network.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,8 @@ async def _inner_fulfill(
445445
encoded_body = base64.b64encode(body).decode()
446446
is_base64 = True
447447
length = len(body)
448+
elif body is not None:
449+
raise Error(f"body must be a str or bytes, got {type(body).__name__}")
448450
elif path:
449451
file_content = Path(path).read_bytes()
450452
encoded_body = base64.b64encode(file_content).decode()

‎playwright/_impl/_protocol_channels.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
from typing import TYPE_CHECKING, Any, Callable, Dict, Literal, Optional, Sequence, Type
3030

31-
from playwright._impl._connection import Channel
31+
from playwright._impl._channel import Channel
3232

3333
if TYPE_CHECKING:
3434
from playwright._impl import _protocol_types as channels

‎playwright/_impl/_protocol_types.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
if TYPE_CHECKING:
4646
from typing_extensions import NotRequired
4747

48-
from playwright._impl._connection import Channel
48+
from playwright._impl._channel import Channel
4949
from playwright._impl._protocol_channels import (
5050
AndroidChannel,
5151
AndroidDeviceChannel,

‎playwright/_impl/_set_input_files_helpers.py‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,25 +83,30 @@ async def convert_input_files(
8383
(_list_files(local_directory) if local_directory else local_paths),
8484
)
8585
streams = []
86-
root_dir_name = (
87-
os.path.basename(local_directory) if local_directory else None
88-
)
89-
temp_files: List[channels.BrowserContextCreateTempFilesParamsItems] = [
90-
{
91-
"name": (
92-
os.path.relpath(file, local_directory)
93-
if local_directory
94-
else os.path.basename(file)
86+
87+
async def create_temp_files() -> (
88+
"channels.BrowserContextCreateTempFilesResult"
89+
):
90+
return await context._channel.createTempFiles(
91+
rootDirName=(
92+
os.path.basename(local_directory) if local_directory else None
9593
),
96-
"lastModifiedMs": int(os.path.getmtime(file) * 1000),
97-
}
98-
for file in files_to_stream
99-
]
94+
items=[
95+
{
96+
"name": (
97+
os.path.relpath(file, local_directory)
98+
if local_directory
99+
else os.path.basename(file)
100+
),
101+
"lastModifiedMs": int(os.path.getmtime(file) * 1000),
102+
}
103+
for file in files_to_stream
104+
],
105+
)
106+
100107
result: channels.BrowserContextCreateTempFilesResult = (
101108
await context._connection.wrap_api_call(
102-
lambda: context._channel.createTempFiles(
103-
rootDirName=root_dir_name, items=temp_files
104-
)
109+
create_temp_files, is_internal=True
105110
)
106111
)
107112
for i, file in enumerate(result["writableStreams"]):

‎scripts/codegen/generate_protocol.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ def generate_types(protocol: Dict[str, Any]) -> Tuple[str, List[Interface], Set[
358358
"if TYPE_CHECKING:",
359359
" from typing_extensions import NotRequired",
360360
"",
361-
" from playwright._impl._connection import Channel",
361+
" from playwright._impl._channel import Channel",
362362
" from playwright._impl._protocol_channels import (",
363363
*[f" {channel}," for channel in used_channels],
364364
" )",
@@ -461,7 +461,7 @@ def generate_channels(interfaces: List[Interface], type_names: Set[str]) -> str:
461461
"",
462462
f"from typing import {', '.join(sorted(typing_names))}",
463463
"",
464-
"from playwright._impl._connection import Channel",
464+
"from playwright._impl._channel import Channel",
465465
"",
466466
"if TYPE_CHECKING:",
467467
" from playwright._impl import _protocol_types as channels",

0 commit comments

Comments
 (0)