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

fix(generic): Also catch URLError waiting for ServerContainer #743

Conversation

mmwinther
Copy link
Contributor

Problem summary

There was a race condition here because the server hasn't always started before we send a request to it. This was causing a lot of unnecessary test failures.

This change makes use of ServerContainer much more reliable.

Example Traceback

../../../Library/Caches/pypoetry/virtualenvs/<redacted>/lib/python3.12/site-packages/testcontainers/generic/server.py:70: in start
    self._connect()
../../../Library/Caches/pypoetry/virtualenvs/<redacted>/lib/python3.12/site-packages/testcontainers/core/waiting_utils.py:59: in wrapper
    return wrapped(*args, **kwargs)
../../../Library/Caches/pypoetry/virtualenvs/<redacted>/lib/python3.12/site-packages/testcontainers/generic/server.py:48: in _connect
    with urlopen(url) as r:
/usr/local/Cellar/[email protected]/3.12.7/Frameworks/Python.framework/Versions/3.12/lib/python3.12/urllib/request.py:215: in urlopen
    return opener.open(url, data, timeout)
/usr/local/Cellar/[email protected]/3.12.7/Frameworks/Python.framework/Versions/3.12/lib/python3.12/urllib/request.py:515: in open
    response = self._open(req, data)
/usr/local/Cellar/[email protected]/3.12.7/Frameworks/Python.framework/Versions/3.12/lib/python3.12/urllib/request.py:532: in _open
    result = self._call_chain(self.handle_open, protocol, protocol +
/usr/local/Cellar/[email protected]/3.12.7/Frameworks/Python.framework/Versions/3.12/lib/python3.12/urllib/request.py:492: in _call_chain
    result = func(*args)
/usr/local/Cellar/[email protected]/3.12.7/Frameworks/Python.framework/Versions/3.12/lib/python3.12/urllib/request.py:1373: in http_open
    return self.do_open(http.client.HTTPConnection, req)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <urllib.request.HTTPHandler object at 0x1247ea3c0>
http_class = <class 'http.client.HTTPConnection'>
req = <urllib.request.Request object at 0x1247e8c80>, http_conn_args = {}
host = '192.168.106.2:32876'
h = <http.client.HTTPConnection object at 0x1247ea360>
headers = {'Connection': 'close', 'Host': '192.168.106.2:32876', 'User-Agent': 'Python-urllib/3.12'}
    def do_open(self, http_class, req, **http_conn_args):
        """Return an HTTPResponse object for the request, using http_class.
    
        http_class must implement the HTTPConnection API from http.client.
        """
        host = req.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({k: v for k, v in req.headers.items()
                        if k not in headers})
    
        # TODO(jhylton): Should this be redesigned to handle
        # persistent connections?
    
        # 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 = {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:
            try:
                h.request(req.get_method(), req.selector, req.data, headers,
                          encode_chunked=req.has_header('Transfer-encoding'))
            except OSError as err: # timeout error
>               raise URLError(err)
E               urllib.error.URLError: <urlopen error [Errno 61] Connection refused>
/usr/local/Cellar/[email protected]/3.12.7/Frameworks/Python.framework/Versions/3.12/lib/python3.12/urllib/request.py:1347: URLError

There was a race condition here because the server
hasn't always started before we send a request to
it. This was causing a lot of unnecessary test failures.
@mmwinther mmwinther changed the title Also catch URLError waiting for ServerContainer fix(generic): Also catch URLError waiting for ServerContainer Nov 26, 2024
Copy link
Member

@alexanderankin alexanderankin left a comment

Choose a reason for hiding this comment

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

Lgtm

@alexanderankin alexanderankin merged commit 24e354f into testcontainers:main Nov 26, 2024
13 of 14 checks passed
alexanderankin pushed a commit that referenced this pull request Dec 10, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.9.0](testcontainers-v4.8.2...testcontainers-v4.9.0)
(2024-11-26)


### Features

* **compose:** support for setting profiles
([#738](#738))
([3e00e71](3e00e71))
* **core:** Support working with env files
([#737](#737))
([932ee30](932ee30))


### Bug Fixes

* allow running all tests
([#721](#721))
([f958cf9](f958cf9))
* **core:** Avoid hanging upon bad docker host connection
([#742](#742))
([4ced198](4ced198))
* **core:** running testcontainer inside container
([#714](#714))
([85a6666](85a6666))
* **generic:** Also catch URLError waiting for ServerContainer
([#743](#743))
([24e354f](24e354f))
* update wait_for_logs to not throw on 'created', and an optimization
([#719](#719))
([271ca9a](271ca9a))
* Vault health check
([#734](#734))
([79434d6](79434d6))


### Documentation

* Documentation fix for ServerContainer
([#671](#671))
([0303d47](0303d47))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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 this pull request may close these issues.

2 participants