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

Client: _request.timeout cancels the current AND the next request #1879

Closed
seperman opened this issue May 9, 2017 · 4 comments
Closed

Client: _request.timeout cancels the current AND the next request #1879

seperman opened this issue May 9, 2017 · 4 comments
Labels

Comments

@seperman
Copy link

seperman commented May 9, 2017

Long story short

_request.timeout will cancel the current request AND the next request.

Expected behaviour

I expected it to behave the same way as aiohttp.Timeout. In other words it should not leak the cancellation to the next request.

Actual behaviour

  • _request.timeout will call TimeoutHandle.__call__ which runs TimeoutHandle._callbacks which runs TimerContext.timeout which sets self._cancelled=True and somehow it leads to the cancellation of the next call of the coroutine too.

  • aiohttp.Timeout does NOT call TimeoutHandle.__call__

Steps to reproduce

Run a local server on port 8888 (I left a sample code for it below). Make it slow to cause timeout errors from the client. In this example 2 seconds cause timeout error.
In order to see the difference between the behaviors, switch from fetch = fetch_cancells to fetch = fetch_works and vice verca.

Client

import asyncio
import aiohttp


async def fetch_cancells(session, url):
    async with session.get(url, timeout=2) as response:
        return response.status, await response.text()


async def fetch_works(session, url):
    with aiohttp.Timeout(2, loop=session.loop):
        async with session.get(url) as response:
            return response.status, await response.text()

# Change this line to fetch=fetch_works to see it working.
fetch = fetch_cancells
# fetch = fetch_works
print(fetch.__name__)


async def hit_url(loop):
    print('Hitting')
    async with aiohttp.ClientSession(loop=loop) as session:
        status, body = await fetch(session, 'http://localhost:8888')
        return status, body


async def master(loop):
    print('----------')
    while True:
        try:
            status, body = await hit_url(loop)
        except asyncio.CancelledError:
            print('Cancelled!')
        except asyncio.TimeoutError:
            print('Time Out!!')
        except Exception:
            raise
        else:
            print(status)
            print(body)
        # await asyncio.sleep(.5)  # <--


def main():
    loop = asyncio.get_event_loop()
    loop.set_debug(True)
    try:
        loop.run_until_complete(master(loop))
    finally:
        loop.close()


if __name__ == '__main__':
    main()

Results

This is what you get with fetch = fetch_works, aka expected behavior:

fetch_works
----------
Hitting
TimerContext._cancelled=False
Time Out!!
----------
Hitting
TimerContext._cancelled=False
Time Out!!
----------
Hitting
TimerContext._cancelled=False
Time Out!!

And this is when fetch=fetch_cancells

fetch_cancells
----------
Hitting
TimeoutHandle.__call__ is called
TimerContext.timeout is called
TimerContext._cancelled=True
TimerContext._cancelled=True
Time Out!!
----------
Hitting
TimerContext._cancelled=False
Cancelled!
----------
Hitting
TimeoutHandle.__call__ is called
TimerContext.timeout is called
TimerContext._cancelled=True
TimerContext._cancelled=True
Time Out!!
----------
Hitting
TimerContext._cancelled=False
Cancelled!

Basically with every new hit, the response alternates between Timeout and Cancelled instead of only staying on Timeout.

The sample server used

In case it helps, here is the server I used:

#!/usr/bin/env python
"""
Very simple HTTP server in python.

Usage::
    ./dummy-web-server.py [<port>]

Send a GET request::
    curl http://localhost

Send a HEAD request::
    curl -I http://localhost

Send a POST request::
    curl -d "foo=bar&bin=baz" http://localhost

"""
from http.server import BaseHTTPRequestHandler, HTTPServer


class Server(BaseHTTPRequestHandler):
    def _set_headers(self):
        self.send_response(200)
        self.send_header(b'Content-type', 'text/html')
        self.end_headers()

    def do_GET(self):
        import pdb; pdb.set_trace()
        self._set_headers()
        self.wfile.write(b"<html><body><h1>hi!</h1></body></html>")

    def do_HEAD(self):
        self._set_headers()

    def do_POST(self):
        # Doesn't do anything with posted data
        self._set_headers()
        self.wfile.write(b"<html><body><h1>POST!</h1></body></html>")


def run(server_class=HTTPServer, handler_class=Server, port=8888):
    server_address = ('', port)
    httpd = server_class(server_address, handler_class)
    print('Starting httpd...')
    httpd.serve_forever()


if __name__ == "__main__":
    from sys import argv

    if len(argv) == 2:
        run(port=int(argv[1]))
    else:
        run()

Your environment

  • MacOs Sierra
  • Python 3.6.0
  • Aiohttp 2.0.7
@fafhrd91
Copy link
Member

thanks for report! should be fixed in master

@seperman
Copy link
Author

Cool. Thanks!

@gjthompson1
Copy link

gjthompson1 commented Aug 21, 2017

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants