false-negative indicator end_of_HTTP_chunk in readchunk #3362
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3362 +/- ##
==========================================
- Coverage 98.01% 97.97% -0.04%
==========================================
Files 44 44
Lines 8396 8402 +6
Branches 1374 1376 +2
==========================================
+ Hits 8229 8232 +3
- Misses 69 71 +2
- Partials 98 99 +1
Continue to review full report at Codecov.
|
|
tests added |
|
I can find a time tomorrow only |
|
ok. thanks |
|
Deferring review to @asvetlov |
|
@asvetlov ping |
|
On my radar |
|
Perfect! Thanks! |
|
@mnach the PR introduces the #3362 bug FYI |
|
Streams code is too complex to fit in my brain. |
|
@asvetlov Everything covered with tests. Please do not revert. We thoroughly tested it during bug fixing. Yes, it is complex. Yes it should be re-implemented. But do not revert, maybe I will help |
|
With the PR the following snippet never passes all 100 iterations. If you or @mnach want to work on it -- it would be fine. import aiohttp
import asyncio
import sys
async def main():
async with aiohttp.ClientSession() as sess:
for i in range(100):
async with sess.get('https://api.scryfall.com/cards/random') as resp:
ret = await resp.json()
if ret is None:
print('Fail', i)
return 123
return 0
sys.exit(asyncio.run(main())) |
|
Yes, I want to work on this bproblem. |
|
Perfect! |
with SSL-related traceback ? or just 123 ? |
|
It depends: |
|
Some debug: Seems, bug happen when we receive chunked, gzipped content, and when a chunk contains some gzipped data, that is not resolved to any user-visible data since that data is not enough for gzip decoder FSM to output anything. Seems code is not ready for situation, when some chunk arrived, but no data added to the internal buffer for reading. Continuing investigation. |
|
The problem: we can not distiguish between last empty chunk, and generic chunk that yields no data when gzip is used. (!) that's why everything fail. @asvetlov unrelated problem: why |
|
Are you sure about gzipped content? I don't see |
|
|
|
@socketpair we can don't emit empty chunks if EOF is not reached. |
|
I have to go, but I think I will fix it today. I have enough debug information. |
|
@asvetlov it seems, that server SOMETIMES return gzipped response: Starting request -------------------------------------
client_proto: feed_data: b'HTTP/1.1 200 OK\r\nDate: Fri, 04 Jan 2019 13:39:13 GMT\r\nConnection: keep-alive\r\nX-Frame-Options: DENY\r\nX-Xss-Protection: 1; mode=block\r\nX-Content-Type-Options: nosniff\r\nX-Download-Options: noopen\r\nX-Permitted-Cross-Domain-Policies: none\r\nReferrer-Policy: strict-origin-when-cross-origin\r\nServer: Scryfall API\r\nAccess-Control-Allow-Origin: *\r\nAccess-Control-Allow-Methods: GET, POST, DELETE, OPTIONS\r\nAccess-Control-Allow-Headers: Accept, Accept-Charset, Accept-Language, Authorization, Cache-Control, Content-Language, Content-Type, DNT, Host, If-Modified-Since, Keep-Alive, Origin, Referer, User-Agent, X-Requested-With\r\nAccess-Control-Max-Age: 300\r\nCache-Control: no-cache, no-store\r\nX-Robots-Tag: none\r\nX-Scryfall-Card-Image: https://img.scryfall.com/cards/large/en/w17/18.jpg?1517813031\r\nX-Scryfall-Card: https://scryfall.com/card/w17/18/raise-dead?utm_source=api\r\nContent-Type: application/json; charset=utf-8\r\nContent-Disposition: inline\r\nContent-Transfer-Encoding: binary\r\nVary: Accept-Encoding\r\nContent-Encoding: gzip\r\nStrict-Transport-Security: max-age=31536000; includeSubDomains; preload\r\nTransfer-Encoding: chunked\r\nVia: 1.1 vegur\r\n\r\n' |
|
@socketpair server returns $ while :; do curl -vvv https://api.scryfall.com/cards/random 2>&1 | grep -i gzip; sleep 0.1; done
^C%
$ while :; do curl -vvv -H 'Accept-Encoding: gzip' https://api.scryfall.com/cards/random 2>&1 | grep -i gzip; sleep 0.1; done
> Accept-Encoding: gzip
< Content-Encoding: gzip
> Accept-Encoding: gzip
< Content-Encoding: gzip
> Accept-Encoding: gzip
< Content-Encoding: gzip
^C% |
|
@socketpair cool! |
|
@asvetlov, In the end, after my fix all 100 iterations of your program run without any troubles. |
psf-chronographer
bot
added
the
bot:chronographer:provided
What do these changes do?
fix a race-condition bug #3361 by:
await self._wait('readchunk')whenon_chunk_completecalledreturn ("", True)inreadchunkwhen buffer is empty, but_http_chunk_splitsisn't and the end user already has a full chunk of datapos==cursor(on_chunk_completecalled recently)Are there changes in behavior for the user?
Related issue number
#3361
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.