EmptyStreamReader: infinite iter_chunks() iteration

[dmoklaf]

Describe the bug

An iteration of the chunks of an EmptyStreamReader with iter_chunks() never ends.

To Reproduce

 import aiohttp
 
 r = aiohttp.streams.EmptyStreamReader()
 async for data, end_of_chunk in r.iter_chunks():
     print("CHUNK", len(data), end_of_chunk)

Expected behavior

The iteration should end on its own.
This is due to:

• EmptyStreamReader returning (data=b'', end_of_chunk=True) since Return empty bytes with end-of-chunk marker in empty stream reader #3295 - before it was returning (data=b'', end_of_chunk =False)
• iter_chunks stopping only based on end_of_chunk=False

3 alternative solutions:

1. Fix nothing and just document this odd behavior in iter_chunk's documentation - to have the client of the API add himself a stopping condition on len(data) == 0
   I am however wondering if this condition could occur naturally in a chunked HTTP stream that is not EOF

2. Fix iter_chunks to stop when len(data) == 0
   I am however wondering if this condition could occur naturally in a chunked HTTP stream that is not EOF

3. Revert Return empty bytes with end-of-chunk marker in empty stream reader #3295
   I am unsure about the consequences in other parts of the library (and the client base)

My unexperienced intuition (I don't know all the aiohttp internal code) is that (3) is the right way to go since #3295 introduced a contradiction with

aiohttp/aiohttp/streams.py

Lines 413 to 415 in b96e15c

	# Special case for signifying EOF.
	# (b'', True) is not a final return value actually.
	return (b"", False)

which states explicitly that an EOF is represented by (data=b'', end_of_chunk =False)

and also of course with the ChunkTupleAsyncStreamIterator implementation:

aiohttp/aiohttp/streams.py

Lines 61 to 62 in b96e15c

	if rv == (b"", False):
	raise StopAsyncIteration

Logs/tracebacks

CHUNK 0 True
CHUNK 0 True
CHUNK 0 True
CHUNK 0 True
CHUNK 0 True
CHUNK 0 True
CHUNK 0 True
... (infinite)

Python Version

Python 3.11.5

aiohttp Version

Name: aiohttp
Version: 3.8.5
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /opt/homebrew/Caskroom/mambaforge/base/envs/work/lib/python3.11/site-packages
Requires: aiosignal, async-timeout, attrs, charset-normalizer, frozenlist, multidict, yarl
Required-by: aiohttp-jinja2, aiohttp-socks, datasets, httpstan, openai, pystan

multidict Version

Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /opt/homebrew/Caskroom/mambaforge/base/envs/work/lib/python3.11/site-packages
Requires: 
Required-by: aiohttp, yarl

yarl Version

Name: yarl
Version: 1.9.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Location: /opt/homebrew/Caskroom/mambaforge/base/envs/work/lib/python3.11/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

macOS 13.5.2
MacBook Pro 16" M2

Related component

Client

Additional context

No response

Code of Conduct

• I agree to follow the aio-libs Code of Conduct

[Dreamsorcerer]

I'm not entirely sure, but I think it should probably return (b'', True) then (b'', False). i.e. 1 empty chunk, then EOF.

[dmoklaf]

Yep I came to the same conclusion, that would check all the boxes

[Dreamsorcerer]

Well, if you could create a PR fixing it, with a simple test that reproduces the iteration failure, that'd be great.

[mind1m]

Looks like this can be closed

[Dreamsorcerer]

Ah, didn't put 'fixes' in the PR description.