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

_on_chunk_sent at write_eof #2909

Closed
vasilenkoalexey opened this issue Apr 3, 2018 · 6 comments
Closed

_on_chunk_sent at write_eof #2909

vasilenkoalexey opened this issue Apr 3, 2018 · 6 comments
Labels

Comments

@vasilenkoalexey
Copy link

Long story short

Looks like _on_chunk_sent not called when last chunk sent at write_eof

Expected behaviour

_on_chunk_sent called

Actual behaviour

_on_chunk_sent not called

Steps to reproduce

at http_writer.py

async def write(self, chunk, *, drain=True, LIMIT=0x10000):
...
    if self._on_chunk_sent is not None:
        await self._on_chunk_sent(chunk)
...
    if chunk:
        if self.chunked:
            chunk_len = ('%x\r\n' % len(chunk)).encode('ascii')
            chunk = chunk_len + chunk + b'\r\n'

        self._write(chunk)

but

async def write_eof(self, chunk=b''):
...
     if chunk:
         self._write(chunk)

looks like last chunk has been sent and there is not a single call _on_chunk_sent

Your environment

Python 3.6
aiohttp 3.1.1
Windows\Linux

@pfreixes
Copy link
Contributor

pfreixes commented Apr 3, 2018

Yes, looks like a bug.

do you feel strong enough to make a MR with the bugfix and the proper unit test? should be easy.

@vasilenkoalexey
Copy link
Author

vasilenkoalexey commented Apr 4, 2018

OMG, fixed faster than I was able to answer. Cool, thank you.
I'm not sure how it matters, but at write function self._on_chunk_sent(chunk) called before chunk = self._compress.compress(chunk), and as i see at fixed write_eof function self._on_chunk_sent(chunk) called after chunk = self._compress.compress(chunk). Probably the logic should be the same in both functions.

@pfreixes
Copy link
Contributor

pfreixes commented Apr 4, 2018

Yes, it matters. For sake of consistency, both methods should give as a param the chunk value at the same stage. I will fix it in master and then I will do the cherry picking to 3.1.

Thanks!

@vasilenkoalexey
Copy link
Author

Can be closed?

@asvetlov
Copy link
Member

Yes, sure.
Thanks for the appointment.

@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