Skip to content

Response.content retains and re-raises errors from a broken stream#7585

Open
agu2347 wants to merge 1 commit into
psf:mainfrom
agu2347:fix-content-retains-error
Open

Response.content retains and re-raises errors from a broken stream#7585
agu2347 wants to merge 1 commit into
psf:mainfrom
agu2347:fix-content-retains-error

Conversation

@agu2347

@agu2347 agu2347 commented Jul 18, 2026

Copy link
Copy Markdown

Fixes #4965.

Previously, if reading response.content raised an exception (e.g. ChunkedEncodingError from an incomplete chunked response), the exception propagated on the first access, but _content stayed at its initial sentinel value (False) and _content_consumed stayed False. A second access to .content would then attempt to re-read the underlying stream. Since the connection is often already dead after the first failure, this second attempt silently "succeeded" with no data, so .content returned b'' instead of raising -- masking the original error entirely, as described in the issue.

Fix: add a _content_error attribute that stores the exception raised while reading content. If set, .content re-raises it on every subsequent access instead of attempting (and silently succeeding at) reading an already-broken stream again.

Verification: added a regression test using a stateful fake raw object that faithfully reproduces the real urllib3 behavior being worked around -- raises once on the first .stream() call, then yields no further data (no error) on a second call, simulating a dead connection. I confirmed the test fails without the fix (second access returns b'' instead of raising) and passes with it.

Ran the existing non-network-dependent test suite; results are identical to a clean checkout of main aside from the one new passing test (pre-existing httpbin fixture and network-dependent failures/errors in this sandbox are present on a clean main checkout too, unrelated to this change).

Previously, if reading response.content raised an exception (e.g.
ChunkedEncodingError from an incomplete chunked response), the
exception propagated on the first access, but _content stayed at its
initial sentinel value (False) and _content_consumed stayed False.
A second access to .content would then attempt to re-read the
underlying stream. Since the connection/stream is often already dead
after the first failure, this second attempt silently succeeded with
no data, so .content returned b'' instead of raising -- masking the
original error entirely.

Add a _content_error attribute that stores the exception raised while
reading content. If set, .content re-raises it on every subsequent
access instead of attempting (and silently succeeding at) reading an
already-broken stream again.

Added a regression test using a stateful fake raw object that
faithfully reproduces the real urllib3 behavior being worked around:
raises once on the first stream() call, then yields no further data
(no error) on a second call, simulating a dead connection. Confirmed
the test fails without the fix (second access returns empty bytes
instead of raising) and passes with it. Ran the existing
non-network-dependent test suite; results are identical to a clean
checkout of main aside from the one new passing test (pre-existing
httpbin fixture and network-dependent failures/errors in this sandbox
are present on a clean main checkout too, unrelated to this change).

Fixes psf#4965
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accessing response.content twice removes forgets read error

1 participant