alfred82santa / whalesong

Whalesong is an asyncio python library to manage WebApps remotely. Currently WhatsappWeb is implemented
https://whalesong.readthedocs.io
MIT License
50 stars 19 forks source link

Calling driver.stop() throws up selenium exception #82

Closed parthibd closed 5 years ago

parthibd commented 5 years ago

Here is my test script


import asyncio
import os

import whalesong

BASE_DIR = os.path.dirname(os.path.abspath(__file__))
PROFILE_PATH = os.path.join(BASE_DIR, 'test-profile')
driver: whalesong.Whalesong = whalesong.Whalesong(
    profile=PROFILE_PATH,
    loadstyles=True,
)

async def start():
    await driver.start()
    await driver.stop()

if __name__ == '__main__':
    driver.loop.run_until_complete(start())

And here the exception

selenium.common.exceptions.InvalidSessionIdException: Message: Tried to run command without establishing a connection

alfred82santa commented 5 years ago

Please add exception traceback

parthibd commented 5 years ago
Traceback (most recent call last):
  File "C:\Program Files\JetBrains\PyCharm 2018.2.5\helpers\pydev\pydevd.py", line 1664, in <module>
    main()
  File "C:\Program Files\JetBrains\PyCharm 2018.2.5\helpers\pydev\pydevd.py", line 1658, in main
    globals = debugger.run(setup['file'], None, None, is_module)
  File "C:\Program Files\JetBrains\PyCharm 2018.2.5\helpers\pydev\pydevd.py", line 1068, in run
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "C:\Program Files\JetBrains\PyCharm 2018.2.5\helpers\pydev\_pydev_imps\_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "C:/Users/Parthib/PycharmProjects/whatsapp-api/test.py", line 20, in <module>
    driver.loop.run_until_complete(start())
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\asyncio\base_events.py", line 584, in run_until_complete
    return future.result()
  File "C:/Users/Parthib/PycharmProjects/whatsapp-api/test.py", line 16, in start
    await driver.stop()
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\whalesong\__init__.py", line 101, in stop
    await self._driver.close()
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\whalesong\driver.py", line 144, in close
    await self._internal_close()
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\whalesong\driver_firefox.py", line 162, in _internal_close
    await self._run_async(self.driver.close)
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\whalesong\driver_firefox.py", line 60, in _run_async
    return await self.loop.run_in_executor(self._pool_executor, partial(method, *args, **kwargs))
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\concurrent\futures\thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 688, in close
    self.execute(Command.CLOSE)
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 321, in execute
    self.error_handler.check_response(response)
  File "C:\Users\Parthib\AppData\Local\Programs\Python\Python37-32\lib\site-packages\selenium\webdriver\remote\errorhandler.py", line 242, in check_response
    raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.InvalidSessionIdException: Message: Tried to run command without establishing a connection
parthibd commented 5 years ago

capture

parthibd commented 5 years ago

https://github.com/alfred82santa/whalesong/blob/cbbbb2141c3008f44ba5e81adde989b47cd79096/whalesong/driver_firefox.py#L162

Why is close() called twice ?

parthibd commented 5 years ago

I this this is the root of the problem.

parthibd commented 5 years ago

https://github.com/alfred82santa/whalesong/blob/cbbbb2141c3008f44ba5e81adde989b47cd79096/whalesong/driver_firefox.py#L177

I think the exception is because when I explicitly call close() the connection is closed but when the polling driver calls close() it raises an exception since the connection already tore down.

parthibd commented 5 years ago

I think it can be fixed if we check the webdriver status before calling close() . Is there an API for that ?

alfred82santa commented 5 years ago

Yes, I think we are calling close twice, too. As workaround you could catch exception.

parthibd commented 5 years ago

Beside catching that exception is there a way we can check the status of webdriver status so that we don't land in the exception in the first place ?

parthibd commented 5 years ago

I did this


            try:
                self.driver.quit()
                await self.close()
            except ConnectionRefusedError:
                pass
            except Exception as e:
                pass
parthibd commented 5 years ago

Is this the correct way ? If it is I will create a pull request

alfred82santa commented 5 years ago

Well, I don't think it is a good solution, it's a workaround. Good solution must be something more generalist. I think it must be on abstract class, it must avoid to try to stop driver when it is stopping or stopped, or to try to start when it is starting or already started.

parthibd commented 5 years ago

what if I do this


            try:
                if not self._fut_running.done():
                    await self.close()
            except ConnectionRefusedError:
                pass
            except WebDriverException:
                pass
            except Exception:
                pass
parthibd commented 5 years ago

Do you think it's still a workaround ?

alfred82santa commented 5 years ago

I mean:

this method should be: (https://github.com/alfred82santa/whalesong/blob/master/whalesong/driver.py#L40)

    async def start_driver(self):
        if self._fut_stop is not None:
            await self._fut_stop
            self._fut_stop = None

        if self._fut_start:
            await self._fut_start
            return

        self._fut_start = ensure_future(self._internal_start_driver())
        await self._fut_start

And this other should be: (https://github.com/alfred82santa/whalesong/blob/master/whalesong/driver.py#L143)

    async def close(self):
         if self._fut_start is not None:
            await self._fut_start
         else:
            return # Not started

         if self._fut_stop:
            await self._fut_stop
            return

        self._fut_stop = ensure_future(self._internal_close())
        await self._fut_stop
        self._fut_start = None

Try this change, and if it works I'll approve a PR.

parthibd commented 5 years ago

The problem is the await


        if self._fut_start is not None:
            await self._fut_start

It just waits there indefinitely

parthibd commented 5 years ago

If I change the order to


        self._fut_start = None
        self._fut_stop = ensure_future(self._internal_close())
        await self._fut_stop

your fix works . seems like a race condition. I am not so sure

alfred82santa commented 5 years ago

Change this: (https://github.com/alfred82santa/whalesong/blob/master/whalesong/driver_firefox.py#L177)

            try:
                await self.close()
            except ConnectionRefusedError:
                pass

by:

            ensure_future(self.close())