Skip to content

[internal] hang bug in PlaywrightContextManager and Transport #650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
x0day opened this issue Apr 26, 2021 · 0 comments · Fixed by #672
Closed

[internal] hang bug in PlaywrightContextManager and Transport #650

x0day opened this issue Apr 26, 2021 · 0 comments · Fixed by #672

Comments

@x0day
Copy link
Contributor

x0day commented Apr 26, 2021

with recently feature(pull/630), playwright-python can connect to remote websocket endpoint, but with this feature, a bug become obvious.

playwright-python doesn't handle the transport's connection error during the connecting, if error found during the connecting. the task will be hang, because below code will never return.

https://github.com/microsoft/playwright-python/blob/master/playwright/async_api/_context_manager.py#L37

self._connection.wait_for_object_with_known_name("Playwright")

eg:

Connect call failed in WebSocketTransport.

maybe we should wait the obj and Transport.on_error_future same time in PlaywrightContextManager like this:

class WebSocketTransport(AsyncIOEventEmitter, Transport):
    ...
    async def run(self) -> None:
        ...

        try:
            self._connection = await websockets.connect(self.ws_endpoint, **options)
        except Exception as exc:
            self.emit("close")
            self.on_error_future.set_exception(exc)
       ...
class PlaywrightContextManager:
    ...
    async def __aenter__(self) -> AsyncPlaywright:
        ...
        done, pending = await asyncio.wait(
            {
                self._connection._transport.on_error_future,
                self._connection.wait_for_object_with_known_name("Playwright"),
            },
            return_when=asyncio.FIRST_COMPLETED,
        )
        obj = next(iter(done)).result()
        playwright = AsyncPlaywright(obj)
        ...
@mxschmitt mxschmitt changed the title hang bug in PlaywrightContextManager and Transport [internal] hang bug in PlaywrightContextManager and Transport Apr 26, 2021
pavelfeldman added a commit to pavelfeldman/playwright-python that referenced this issue May 7, 2021
) (microsoft#651)"

This reverts commit 82885a1.
Sorry about the revert. I don't think this handles the connection
timeout properly, so let me see if we can get a better fix here.
pavelfeldman added a commit that referenced this issue May 7, 2021
…" (#670)

This reverts commit 82885a1.
Sorry about the revert. I don't think this handles the connection
timeout properly, so let me see if we can get a better fix here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants