HyperionGray / trio-chrome-devtools-protocol

Trio driver for Chrome DevTools Protocol (CDP)
MIT License
60 stars 17 forks source link

examples failing with ConnectionClosed exception #5

Closed phaustin closed 4 years ago

phaustin commented 4 years ago

Hi -- I'm running the screenshot and get_title examples and they are both succeeding, but when exiting the context block raise an exception:

% python screenshot.py ws://127.0.0.1:9000/devtools/browser/91c7b198-5af0-42d4-8e37-cf87ff880dd2 https://google.com
INFO:screenshot:Connecting to browser: ws://127.0.0.1:9000/devtools/browser/91c7b198-5af0-42d4-8e37-cf87ff880dd2
INFO:screenshot:Listing targets
INFO:screenshot:Attaching to target id=82C8C3F0A2F7DF1F6363F23BCEFC0E7D
INFO:screenshot:Setting device emulation
INFO:screenshot:Enabling page events
INFO:screenshot:Navigating to https://google.com
INFO:screenshot:Making a screenshot
INFO:screenshot:Saving to file
Traceback (most recent call last):
  File "screenshot.py", line 73, in <module>
    trio.run(main, restrict_keyboard_interrupt_to_checkpoints=True)
  File "/Users/phil/repos/trio/trio/_core/_run.py", line 1804, in run
    raise runner.main_task_outcome.error
  File "screenshot.py", line 66, in main
    await screenshot_file.write(b64decode(img_data))
  File "/Users/phil/a50037/envs/cdp/lib/python3.7/contextlib.py", line 177, in __aexit__
    await self.gen.__anext__()
  File "/Users/phil/repos/trio-chrome-devtools-protocol/trio_cdp/__init__.py", line 307, in open_cdp_connection
    yield cdp_conn
  File "/Users/phil/repos/trio/trio/_core/_run.py", line 730, in __aexit__
    raise combined_error_from_nursery
  File "/Users/phil/repos/trio-chrome-devtools-protocol/trio_cdp/__init__.py", line 208, in _reader_task
    message = await self.ws.get_message()
  File "/Users/phil/a50037/envs/cdp/lib/python3.7/site-packages/trio_websocket/_impl.py", line 823, in get_message
    raise ConnectionClosed(self._close_reason) from None
trio_websocket._impl.ConnectionClosed: None
auphofBSF commented 4 years ago

Hello, unfortunately I am also getting the same issue, I have activated debug on trio-websockets and show the following final lines logging of examples/get_title.py The async with open_cdp_connection(......) as conn: exit seems to terminate the receive and then we receive the websocket closed ConnectionClosed<CloseReason<code=1006, name=ABNORMAL_CLOSURE, reason=None

Versions

python 3.7.6 trio-chrome-devtools-protocol 0.4.0 (trip_cdp) - from github master 905b3a93a441ffc9e7e4dc896ac0ea54d4f7f906 trio-websocket 0.8.0 chrome 69.0.3497.128

Logging excerpt

DEBUG:trio_cdp:Received message {'id': 4, 'result': {'outerHTML': '<title>Example Domain</title>'}, 'sessionId': '424FC2BC6F78F6222305AAA337285080'}
<title>Example Domain</title>
DEBUG:trio-websocket:client-0 sending 8 bytes
DEBUG:trio-websocket:client-0 received zero bytes (connection closed)
DEBUG:trio-websocket:client-0 websocket closed ConnectionClosed<CloseReason<code=1006, name=ABNORMAL_CLOSURE, reason=None>>
DEBUG:trio-websocket:client-0 reader task finished
Traceback (most recent call last):
  File "c:\Users\<USER>\.vscode\extensions\ms-python.python-2020.2.64397\pythonFiles\ptvsd_launcher.py", line 48, in <module>
    main(ptvsdArgs)
  File "c:\Users\<USER>\.vscode\extensions\ms-python.python-2020.2.64397\pythonFiles\lib\python\old_ptvsd\ptvsd\__main__.py", line 432, in main
    run()
  File "c:\Users\<USER>\.vscode\extensions\ms-python.python-2020.2.64397\pythonFiles\lib\python\old_ptvsd\ptvsd\__main__.py", line 316, in run_file
    runpy.run_path(target, run_name='__main__')
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\runpy.py", line 263, in run_path
    pkg_name=pkg_name, script_name=fname)
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\runpy.py", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "c:\Users\<USER>\Cloudstation\<DIR1>\github\trio-chrome-devtools-protocol\examples\get_title.py", line 63, in <module>
    trio.run(main, restrict_keyboard_interrupt_to_checkpoints=True)
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\site-packages\trio\_core\_run.py", line 1804, in run
    raise runner.main_task_outcome.error
  File "c:\Users\<USER>\Cloudstation\<DIR1>\github\trio-chrome-devtools-protocol\examples\get_title_simple_calls.py", line 64, in main
    print(html)
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\contextlib.py", line 177, in __aexit__
    await self.gen.__anext__()
  File "c:\users\<USER>\cloudstation\<DIR1>\github\trio-chrome-devtools-protocol\trio_cdp\__init__.py", line 309, in open_cdp_connection
    yield cdp_conn
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\site-packages\trio\_core\_run.py", line 730, in __aexit__
    raise combined_error_from_nursery
  File "c:\users\<USER>\cloudstation\<DIR1>\github\trio-chrome-devtools-protocol\trio_cdp\__init__.py", line 210, in _reader_task
    message = await self.ws.get_message()
  File "C:\Users\<USER>\Anaconda3\envs\<ENV>\lib\site-packages\trio_websocket\_impl.py", line 823, in get_message
    raise ConnectionClosed(self._close_reason) from None
trio_websocket._impl.ConnectionClosed: None
mehaase commented 4 years ago

Thank you for reporting this. The same issue seems to have caused build failures since build 20.

~I suspect its a regression in a dependency, because the first commit that broke was "Bump version to 0.4.0", which should not have caused a break like this.~

Edit: it looks like this was broken by a8cbe893c59398c4df5cf3a62394b294d61d5ae1 but I pushed several commits at once and it broke in build 20.

Thank you for the PR @auphofBSF. I'm still looking into it.

mehaase commented 4 years ago

Whoops, I thought this was the same root cause as the build failure, but the examples still fail after the previous commit. Re-opening...

mehaase commented 4 years ago

Interestingly, I cannot reproduce this on MacOS. It only appears on Linux.

auphofBSF commented 4 years ago

With your commit 0af6f338317a013d217b30e7d307d35490c6be36 I also have exception trio_websocket._impl.ConnectionClosed: CloseReason<code=1006, name=ABNORMAL_CLOSURE, reason=None> appearing on Win10 64, so I explored a bit further In this commit https://github.com/auphofBSF/trio-chrome-devtools-protocol/commit/60a8393c96fe8df8fbc6d85d778289233f344043 i have put some debugging help that shows an issue

This showed I was using the new https://github.com/HyperionGray/python-chrome-devtools-protocol/commit/f4525a7746704f5ec2dae16ae2dff66f57699324 and type should be type_ .
The issue being that any errors raised here are not being raised up the stack, ie If I fix to t.type_ ==.... and uncomment the assert False an abnormal closure arises, not an assert failure.

I then compared against my PR #7 and the error for type is being raised correctly and on fixing type to type_ it runs correctly. I am not fully understanding the async nursery scopes and canceling, maybe this has some effect on exceptions raised. Hopefully this info is of some assistance!

My feeling is there has to be some signal to terminate the _reader_task and PR #7 uses the connection closed exception as this mechanism, but I was hoping to raise an Exception on reason being != Normal. I could not get this working?

mehaase commented 4 years ago

I've just pushed a fix for this. The new design is based on trio-websocket, which is one of the dependencies, and this design has worked well in that project. The basic principle is that the reader task should not throw exceptions: if the connection is closed, then the reader task should quietly exit. Instead, the connection closed exception should be raised in the public APIs such as session.execute() and session.listen(). The reasoning behind this is that it's much easier for the caller to catch an exception in the foreground task:

async with trio_cdp.open_cdp('ws://localhost:9000/...') as conn:
    try:
        targets = await conn.execute(target.get_targets())
    except ConnectionClosed:
        print('whoops! connection is closed')

Compare this to trying to catch the ConnectionClosed in a background task:

async with trio_cdp.open_cdp(...) as conn: # <-- Background task raises exception here!
    try:
        targets = await conn.execute(target.get_targets())
    except ConnectionClosed:
        # Background task exception cannot be caught here!
        print('whoops! connection is closed')

Not only does the background task raise its exception in an inconvenient place, but it can also raise MultiError if you have other tasks in the same nursery.

Note that ConnectionClosed is always raised if you try to execute commands on a closed connection, even if the connection reason is "1000/normal". The exception does have a field reason that contains a trio_websocket.CloseReason in case you want to inspect why it was closed. The closure reason is also displayed when you stringify the exception, so it appears automatically in exception logs.

Finally, there is still an open bug related to closing connections and not tearing down sessions correctly. I've opened #8 to address that, but it is less critical than this issue.