Skip to content
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

[py] Fix internal error when an unexpected exception is raised while … #4654

Merged
merged 1 commit into from
Nov 17, 2017

Conversation

carlosgcampos
Copy link
Contributor

This is happening since the switch to use a global driver instance. The
problem is that when a test fails with an exception the
pytest_exception_interact() is called, which quits the driver, and then
the teardown part of the close_windows fixture tries to close the
windows, but window_handles fails because the driver process is no
longer running. We can handle URLError exception when getting the window
handles and simply return without trying to close the windows.

__________________________________________ ERROR at teardown of testClickingOnAButtonThatClosesAnOpenWindowDoesNotCauseTheBrowserToHang[WebKitGTK] ___________________________________________

driver = <selenium.webdriver.webkitgtk.webdriver.WebDriver (session="09a5a90c-32c1-437d-befc-f72dc13540bc")>

    @pytest.fixture(autouse=True)
    def close_windows(driver):
        main_windows_handle = driver.current_window_handle
        yield
>       for handle in driver.window_handles:

test/selenium/webdriver/common/window_switching_tests.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
selenium/webdriver/remote/webdriver.py:629: in window_handles
    return self.execute(Command.W3C_GET_WINDOW_HANDLES)['value']
selenium/webdriver/remote/webdriver.py:306: in execute
    response = self.command_executor.execute(driver_command, params)
selenium/webdriver/remote/remote_connection.py:464: in execute
    return self._request(command_info[0], url, body=data)
selenium/webdriver/remote/remote_connection.py:526: in _request
    resp = opener.open(request, timeout=self._timeout)
/usr/lib/python2.7/urllib2.py:429: in open
    response = self._open(req, data)
/usr/lib/python2.7/urllib2.py:447: in _open
    '_open', req)
/usr/lib/python2.7/urllib2.py:407: in _call_chain
    result = func(*args)
/usr/lib/python2.7/urllib2.py:1228: in http_open
    return self.do_open(httplib.HTTPConnection, req)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <urllib2.HTTPHandler instance at 0x7f34364d04d0>, http_class = <class httplib.HTTPConnection at 0x7f34362146d0>
req = <selenium.webdriver.remote.remote_connection.Request instance at 0x7f34364d07e8>, http_conn_args = {}, host = '127.0.0.1:56969', h = <httplib.HTTPConnection instance at 0x7f34364d01b8>
err = error(111, 'Connection refused')

    def do_open(self, http_class, req, **http_conn_args):
        """Return an addinfourl object for the request, using http_class.
    
            http_class must implement the HTTPConnection API from httplib.
            The addinfourl return value is a file-like object.  It also
            has methods and attributes including:
                - info(): return a mimetools.Message object for the headers
                - geturl(): return the original request URL
                - code: HTTP status code
            """
        host = req.get_host()
        if not host:
            raise URLError('no host given')
    
        # will parse host:port
        h = http_class(host, timeout=req.timeout, **http_conn_args)
        h.set_debuglevel(self._debuglevel)
    
        headers = dict(req.unredirected_hdrs)
        headers.update(dict((k, v) for k, v in req.headers.items()
                            if k not in headers))
    
        # We want to make an HTTP/1.1 request, but the addinfourl
        # class isn't prepared to deal with a persistent connection.
        # It will try to read all remaining data from the socket,
        # which will block while the server waits for the next request.
        # So make sure the connection gets closed after the (only)
        # request.
        headers["Connection"] = "close"
        headers = dict(
            (name.title(), val) for name, val in headers.items())
    
        if req._tunnel_host:
            tunnel_headers = {}
            proxy_auth_hdr = "Proxy-Authorization"
            if proxy_auth_hdr in headers:
                tunnel_headers[proxy_auth_hdr] = headers[proxy_auth_hdr]
                # Proxy-Authorization should not be sent to origin
                # server.
                del headers[proxy_auth_hdr]
            h.set_tunnel(req._tunnel_host, headers=tunnel_headers)
    
        try:
            h.request(req.get_method(), req.get_selector(), req.data, headers)
        except socket.error, err: # XXX what error?
            h.close()
>           raise URLError(err)
E           URLError: <urlopen error [Errno 111] Connection refused>

/usr/lib/python2.7/urllib2.py:1198: URLError

…running window_switching_tests

This is happening since the switch to use a global driver instance. The
problem is that when a test fails with an exception the
pytest_exception_interact() is called, which quits the driver, and then
the teardown part of the close_windows fixture tries to close the
windows, but window_handles fails because the driver process is no
longer running. We can handle URLError exception when getting the window
handles and simply return without trying to close the windows.
from urllib import request as url_request
URLError = url_request.URLError
except ImportError:
import urllib2 as url_request
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this unnecessary import?

@lmtierney
Copy link
Member

@carlosgcampos can you remove the extra import, then I'll merge?

@carlosgcampos
Copy link
Contributor Author

Sure, sorry, I've been busy with other stuff and I forgot about this. I'll check it again next week and push an update. Thanks.

@lmtierney
Copy link
Member

@carlosgcampos don't worry about it, it'll probably be simpler for me to just remove it after merge. Thanks!

@lmtierney lmtierney merged commit 1d39d6b into SeleniumHQ:master Nov 17, 2017
@carlosgcampos carlosgcampos deleted the window-switching-crash branch November 17, 2017 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants