fix(python): raise on mid-stream body errors instead of silent EOF#482
fix(python): raise on mid-stream body errors instead of silent EOF#482barjin wants to merge 2 commits into
Conversation
A mid-stream body error (connection reset, truncated chunked transfer) was mapped to StopIteration/StopAsyncIteration, which signals normal end of iteration. The `async for`/`for` loop ended silently and the caller processed a partial body believing it was complete - a silent data-integrity bug. Now the classified ImpitError is propagated as a real exception, and the consumed/closed flags are set consistently with the clean-EOF branch. Streamed body truncation surfaces in reqwest as a `Decode`-kinded error rather than `Body`, so the existing unexpected-EOF classification missed it and fell through to the catch-all HTTPError. Widen the guard to cover `is_decode()` so truncated streams are reported as RemoteProtocolError, matching httpx.
|
Thank you @Pijukatel , that's a good point there. According to my tests, the behaviour actually differs between While From what I noticed, the I'd leave the final decision on the client maintainers, but imo both the |
Yes, I think both should handle it the same way. To be consistent with JS implementation, we should not raise - at least for the log redirection part. |
|
Maybe something like this will be needed on the Python side: Could you please check that it catches the new impit error? |
Yes, it seems it does (both in the sync and async client) 👍 
Can you please make that branch into a PR? Also, I think we can safely pin Thanks! |
Mid-stream body errors (connection reset, truncated chunked transfer) were mapped to
StopIteration/StopAsyncIteration, which signal normal end-of-iteration — sofor/async forended silently and callers processed partial bodies as complete (a silent data-integrity bug, #475). Both sync and async iterators now propagate the classifiedImpitErroras a real exception and set the consumed/closed flags consistently with the clean-EOF branch.Streamed truncation surfaces in reqwest as a
Decode-kinded error rather thanBody, so the existing unexpected-EOF classification missed it and fell through to the catch-allHTTPError. The guard is widened to coveris_decode(), so truncated streams now raiseRemoteProtocolError, matching httpx.Verified against a vanilla server that truncates the body mid-response; added sync + async regression tests.
Closes #475