From 1ef4f95fe2a5f2f7b854199318c10473ae858cdb Mon Sep 17 00:00:00 2001 From: springmaple Date: Thu, 20 Oct 2016 01:57:28 +0800 Subject: [PATCH 01/16] - Added a function to notify StreamReader when connection is closed. --- aiohttp/client_reqrep.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index c4ff16ed488..d79a56a2d67 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -659,6 +659,7 @@ def close(self): self._connection.close() self._connection = None self._cleanup_writer() + self._notify_content() @asyncio.coroutine def release(self): @@ -682,6 +683,7 @@ def release(self): self._reader.unset_parser() self._connection = None self._cleanup_writer() + self._notify_content() def raise_for_status(self): if 400 <= self.status: @@ -694,6 +696,11 @@ def _cleanup_writer(self): self._writer.cancel() self._writer = None + def _notify_content(self): + if self.content.exception() is None: + self.content.set_exception( + aiohttp.ClientDisconnectedError('Connection closed')) + @asyncio.coroutine def wait_for_close(self): if self._writer is not None: From 307ad92a8eb530503c0f1bac09cf927530aa0f49 Mon Sep 17 00:00:00 2001 From: SpringMaple Date: Thu, 20 Oct 2016 12:29:38 +0800 Subject: [PATCH 02/16] Fix content is NoneType error. --- aiohttp/client_reqrep.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index d79a56a2d67..c8d5f024734 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -697,7 +697,7 @@ def _cleanup_writer(self): self._writer = None def _notify_content(self): - if self.content.exception() is None: + if self.content and self.content.exception() is None: self.content.set_exception( aiohttp.ClientDisconnectedError('Connection closed')) From 33099fc8b092714054660ff4ddd65bb5fd684bd1 Mon Sep 17 00:00:00 2001 From: SpringMaple Date: Thu, 20 Oct 2016 20:18:28 +0800 Subject: [PATCH 03/16] Added unit test. --- tests/test_py35/test_resp.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_py35/test_resp.py b/tests/test_py35/test_resp.py index 79d47232e7b..c8dad6d6ef0 100644 --- a/tests/test_py35/test_resp.py +++ b/tests/test_py35/test_resp.py @@ -96,3 +96,30 @@ async def handler(request): with aiohttp.ClientSession(loop=loop) as session: async with await session.post(server.make_url('/'), data=data) as resp: assert resp.status == 200 + + +async def test_iter_error_on_conn_close(test_server, loop): + + async def handler(request): + resp_ = web.StreamResponse() + await resp_.prepare(request) + for _ in range(3): + resp_.write(b'data\n') + await resp_.drain() + await asyncio.sleep(0.5, loop=loop) + return resp_ + + app = web.Application(loop=loop) + app.router.add_route('GET', '/', handler) + server = await test_server(app) + + with aiohttp.ClientSession(loop=loop) as session: + timer_started = False + async with await session.get( + server.make_url('/'), + headers={'Connection': 'Keep-alive'}) as resp: + with pytest.raises(aiohttp.ClientDisconnectedError): + async for _ in resp.content: + if not timer_started: + loop.call_later(0.5, session.close) + timer_started = True From a3cbd763056fbda91921fcf5848118e056787b91 Mon Sep 17 00:00:00 2001 From: SpringMaple Date: Thu, 20 Oct 2016 20:19:25 +0800 Subject: [PATCH 04/16] Modified CHANGES.rst --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 68305a3f7a4..39271760f87 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -45,7 +45,8 @@ CHANGES - Ensure TestClient HTTP methods return a context manager #1318 -- +- Add a function to notify StreamReader when connection from + ClientSession is closed. #1323 - From d639d12f123372ed2c841b8af035cec5cc8e1adb Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 00:25:37 +0800 Subject: [PATCH 05/16] moved unit test from 'test_py35/test_resp.py' to 'test_client_functional.py' and fix unhandled exception. --- tests/test_client_functional.py | 33 +++++++++++++++++++++++++++++++++ tests/test_py35/test_resp.py | 27 --------------------------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index ee8ca55fb82..89a38fbe007 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -554,6 +554,39 @@ def handler(request): yield from resp.read() +@asyncio.coroutine +def test_iter_error_on_conn_close(loop, test_client): + + @asyncio.coroutine + def handler(request): + resp_ = web.StreamResponse() + yield from resp_.prepare(request) + + # make sure connection is closed by client. + with pytest.raises(aiohttp.ServerDisconnectedError): + for _ in range(10): + resp_.write(b'data\n') + yield from resp_.drain() + yield from asyncio.sleep(0.5, loop=loop) + return resp_ + + app = web.Application(loop=loop) + app.router.add_route('GET', '/', handler) + server = yield from test_client(app) + + with aiohttp.ClientSession(loop=loop) as session: + timer_started = False + url, headers = server.make_url('/'), {'Connection': 'Keep-alive'} + resp = yield from session.get(url, headers=headers) + with resp: + with pytest.raises(aiohttp.ClientDisconnectedError): + for data in resp.content: + assert data.strip() == 'data' + if not timer_started: + loop.call_later(0.5, session.close) + timer_started = True + + @asyncio.coroutine def test_HTTP_200_OK_METHOD(loop, test_client): @asyncio.coroutine diff --git a/tests/test_py35/test_resp.py b/tests/test_py35/test_resp.py index c8dad6d6ef0..79d47232e7b 100644 --- a/tests/test_py35/test_resp.py +++ b/tests/test_py35/test_resp.py @@ -96,30 +96,3 @@ async def handler(request): with aiohttp.ClientSession(loop=loop) as session: async with await session.post(server.make_url('/'), data=data) as resp: assert resp.status == 200 - - -async def test_iter_error_on_conn_close(test_server, loop): - - async def handler(request): - resp_ = web.StreamResponse() - await resp_.prepare(request) - for _ in range(3): - resp_.write(b'data\n') - await resp_.drain() - await asyncio.sleep(0.5, loop=loop) - return resp_ - - app = web.Application(loop=loop) - app.router.add_route('GET', '/', handler) - server = await test_server(app) - - with aiohttp.ClientSession(loop=loop) as session: - timer_started = False - async with await session.get( - server.make_url('/'), - headers={'Connection': 'Keep-alive'}) as resp: - with pytest.raises(aiohttp.ClientDisconnectedError): - async for _ in resp.content: - if not timer_started: - loop.call_later(0.5, session.close) - timer_started = True From e1515a54aa0cc6e0a99612304fc55519c550571c Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 00:42:08 +0800 Subject: [PATCH 06/16] fix unit test error. --- tests/test_client_functional.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 89a38fbe007..787e14afec9 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -578,13 +578,12 @@ def handler(request): timer_started = False url, headers = server.make_url('/'), {'Connection': 'Keep-alive'} resp = yield from session.get(url, headers=headers) - with resp: - with pytest.raises(aiohttp.ClientDisconnectedError): - for data in resp.content: - assert data.strip() == 'data' - if not timer_started: - loop.call_later(0.5, session.close) - timer_started = True + with pytest.raises(aiohttp.ClientDisconnectedError): + for data in resp.content: + assert data.strip() == 'data' + if not timer_started: + loop.call_later(0.5, session.close) + timer_started = True @asyncio.coroutine From 4a902b19db96d912a957c1b2fdace03dc09e1738 Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 00:55:23 +0800 Subject: [PATCH 07/16] fix unit test error. --- tests/test_client_functional.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 787e14afec9..7e74c8eb32a 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -579,8 +579,12 @@ def handler(request): url, headers = server.make_url('/'), {'Connection': 'Keep-alive'} resp = yield from session.get(url, headers=headers) with pytest.raises(aiohttp.ClientDisconnectedError): - for data in resp.content: - assert data.strip() == 'data' + while True: + data = yield from resp.content.readline() + data = data.strip() + if not data: + break + assert data == 'data' if not timer_started: loop.call_later(0.5, session.close) timer_started = True From abed40e0702444e7d3cbe950621266a6c28c0799 Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 01:01:36 +0800 Subject: [PATCH 08/16] fix unit test error. --- tests/test_client_functional.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 7e74c8eb32a..2312127e7a0 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -584,7 +584,7 @@ def handler(request): data = data.strip() if not data: break - assert data == 'data' + assert data == b'data' if not timer_started: loop.call_later(0.5, session.close) timer_started = True From 59645a26d932395c1265590030d1042998a3306e Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 01:15:38 +0800 Subject: [PATCH 09/16] fix unit test error. --- tests/test_client_functional.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 2312127e7a0..252d23eaefa 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -563,12 +563,14 @@ def handler(request): yield from resp_.prepare(request) # make sure connection is closed by client. - with pytest.raises(aiohttp.ServerDisconnectedError): + try: for _ in range(10): resp_.write(b'data\n') yield from resp_.drain() yield from asyncio.sleep(0.5, loop=loop) - return resp_ + except aiohttp.ServerDisconnectedError: + yield from asyncio.sleep(2, loop=loop) + return resp_ app = web.Application(loop=loop) app.router.add_route('GET', '/', handler) From 6bf3b6c188b41b5b67829c8412fc52196ad73dc0 Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 02:23:57 +0800 Subject: [PATCH 10/16] fix unit test error. --- tests/test_client_functional.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 252d23eaefa..8dca84d8e84 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -563,14 +563,12 @@ def handler(request): yield from resp_.prepare(request) # make sure connection is closed by client. - try: + with pytest.raises(aiohttp.ServerDisconnectedError): for _ in range(10): resp_.write(b'data\n') yield from resp_.drain() yield from asyncio.sleep(0.5, loop=loop) - except aiohttp.ServerDisconnectedError: - yield from asyncio.sleep(2, loop=loop) - return resp_ + return resp_ app = web.Application(loop=loop) app.router.add_route('GET', '/', handler) @@ -588,7 +586,7 @@ def handler(request): break assert data == b'data' if not timer_started: - loop.call_later(0.5, session.close) + loop.call_later(1.0, resp.close) timer_started = True From fd1ccac89c4b582004395993e944d60a1b2a1ebc Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 02:26:59 +0800 Subject: [PATCH 11/16] modified contributor list --- CONTRIBUTORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index ded366e6164..db3fdd66a0e 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -134,6 +134,7 @@ Vladimir Rutsky Vladimir Shulyak Vladimir Zakharov Willem de Groot +Wilson Ong W. Trevor King Yannick Koechlin Young-Ho Cha From f845f7551596d7facca7c262fc2d29268a4456df Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 03:09:30 +0800 Subject: [PATCH 12/16] not raising error if eof is read and added unit test for it. --- aiohttp/client_reqrep.py | 5 +-- tests/test_client_functional.py | 57 +++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index c8d5f024734..6f7dc5d62fd 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -697,8 +697,9 @@ def _cleanup_writer(self): self._writer = None def _notify_content(self): - if self.content and self.content.exception() is None: - self.content.set_exception( + content = self.content + if content and content.exception() is None and not content.is_eof(): + content.set_exception( aiohttp.ClientDisconnectedError('Connection closed')) @asyncio.coroutine diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 8dca84d8e84..5061c5b761d 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -555,7 +555,7 @@ def handler(request): @asyncio.coroutine -def test_iter_error_on_conn_close(loop, test_client): +def test_readline_error_on_conn_close(loop, test_client): @asyncio.coroutine def handler(request): @@ -586,10 +586,63 @@ def handler(request): break assert data == b'data' if not timer_started: - loop.call_later(1.0, resp.close) + def do_release(): + loop.create_task(resp.release()) + loop.call_later(1.0, do_release) timer_started = True +@asyncio.coroutine +def test_no_error_on_conn_close_if_eof(loop, test_client): + + @asyncio.coroutine + def handler(request): + resp_ = web.StreamResponse() + yield from resp_.prepare(request) + resp_.write(b'data\n') + yield from resp_.drain() + yield from asyncio.sleep(0.5, loop=loop) + return resp_ + + app = web.Application(loop=loop) + app.router.add_route('GET', '/', handler) + server = yield from test_client(app) + + with aiohttp.ClientSession(loop=loop) as session: + url, headers = server.make_url('/'), {'Connection': 'Keep-alive'} + resp = yield from session.get(url, headers=headers) + while True: + data = yield from resp.content.readline() + data = data.strip() + if not data: + break + assert data == b'data' + yield from resp.release() + assert resp.content.exception() is None + + +@asyncio.coroutine +def test_error_not_overwrote_on_conn_close(loop, test_client): + + @asyncio.coroutine + def handler(request): + resp_ = web.StreamResponse() + yield from resp_.prepare(request) + return resp_ + + app = web.Application(loop=loop) + app.router.add_route('GET', '/', handler) + server = yield from test_client(app) + + with aiohttp.ClientSession(loop=loop) as session: + url, headers = server.make_url('/'), {'Connection': 'Keep-alive'} + resp = yield from session.get(url, headers=headers) + resp.content.set_exception(aiohttp.ClientRequestError()) + + yield from resp.release() + assert isinstance(resp.content.exception(), aiohttp.ClientRequestError) + + @asyncio.coroutine def test_HTTP_200_OK_METHOD(loop, test_client): @asyncio.coroutine From b4e0758e9ca41a284f608c23332b6cc2d4f8a704 Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 03:29:26 +0800 Subject: [PATCH 13/16] add changes to documentation. --- docs/streams.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/streams.rst b/docs/streams.rst index dd1c60445e9..b82f89fbc30 100644 --- a/docs/streams.rst +++ b/docs/streams.rst @@ -71,6 +71,9 @@ Reading Methods If the EOF was received and the internal buffer is empty, return an empty bytes object. + Raise an :exc:`aiohttp.ClientDisconnectedError` if :class:`ClientSession` + object is closed when reading data. + :return bytes: the given line Asynchronous Iteration Support From 8fbb4459280e0ee4171a8df03288199f8e5bad3b Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 03:34:40 +0800 Subject: [PATCH 14/16] modify changes sentence. --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 39271760f87..722a0a04992 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -45,8 +45,8 @@ CHANGES - Ensure TestClient HTTP methods return a context manager #1318 -- Add a function to notify StreamReader when connection from - ClientSession is closed. #1323 +- Raise aiohttp.ClientDisconnectedError to StreamReader if ClientSession + object is closed by client when reading data. #1323 - From 3331b2d8657beb0de0dccca5662c61321edfb11b Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 03:46:36 +0800 Subject: [PATCH 15/16] modify changes sentence. --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 722a0a04992..1e952e30ac3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -45,8 +45,8 @@ CHANGES - Ensure TestClient HTTP methods return a context manager #1318 -- Raise aiohttp.ClientDisconnectedError to StreamReader if ClientSession - object is closed by client when reading data. #1323 +- Raise `ClientDisconnectedError` to `StreamReader` read function if + `ClientSession` object is closed by client when reading data. #1323 - From e865de213690e96e744115f258ffb75afec2f502 Mon Sep 17 00:00:00 2001 From: springmaple Date: Fri, 21 Oct 2016 03:49:46 +0800 Subject: [PATCH 16/16] modify changes sentence. --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1e952e30ac3..11811001f1e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -45,8 +45,8 @@ CHANGES - Ensure TestClient HTTP methods return a context manager #1318 -- Raise `ClientDisconnectedError` to `StreamReader` read function if - `ClientSession` object is closed by client when reading data. #1323 +- Raise `ClientDisconnectedError` to `FlowControlStreamReader` read function + if `ClientSession` object is closed by client when reading data. #1323 -