Fix: retain stream read error on subsequent response.content access

[hellozzm]

Summary

Fixes #4965

When accessing response.content raises an exception during stream reading (e.g., ConnectionError), subsequent accesses would silently return an empty bytestring b"" instead of re-raising the original error.

This made debugging difficult, especially in debuggers where properties may be accessed multiple times.

Changes

• Store the exception in _content_error when iter_content raises during content reading
• On subsequent accesses, re-raise the stored exception

Testing

from unittest.mock import MagicMock
from requests.models import Response

r = Response()
r.status_code = 200
r.raw = MagicMock()
r.raw.stream = MagicMock(side_effect=ConnectionError('reset'))

try:
    _ = r.content
except ConnectionError:
    print('First: raised')  # ✓

try:
    _ = r.content
except ConnectionError:
    print('Second: raised')  # ✓ (previously returned b'')

[golikovichev]

Nice catch on #4965. The silent b"" on the second access is a real foot-gun, especially in a debugger that touches the property more than once.

The fix works. A few suggestions:

Initialising the error: _content_error is set dynamically in the property, whereas Response initialises its state in init (_content = False, _content_consumed = False). It would be more in keeping with the class to add self._content_error = None in init and check if self._content_error is not None, rather than relying on hasattr. That also gives the attribute a place in pickling. Right now it is not in attrs, so it would not round-trip through getstate/setstate, which is probably fine but worth a deliberate choice.

Short-circuit: as written, a second access still re-enters the b"".join(self.iter_content(...)) path before reaching the re-raise, because _content is still False. On an exhausted stream that just yields b"" again, so it is harmless but unnecessary. Moving the if self._content_error is not None: raise to the top of the property would short-circuit a previously failed response right away.

Test: the snippet in the description is a good reproduction. It would be worth landing it as a regression test under tests/, asserting it raises on both the first and the second access. A behaviour change like this usually needs a test, and it guards the b"" regression from coming back.

Minor: except Exception is broad, but since you re-raise immediately it is reasonable here.

[avinashkamat48]

The _content_error check happens after the if self._content is False read attempt, so a later response.content access can still try iter_content() again instead of immediately re-raising the retained original exception. After the first failure _content remains False, so the guard needs to run before the read block, or _content needs to be moved to a sentinel state that skips the retry. A regression test should call .content twice after a streaming read failure and assert the second access raises the original stored exception without invoking the raw stream again.

[golikovichev]

+1, that is the short-circuit point from my review. Since _content stays False after the first failure, the property still re-enters the iter_content join before it re-raises. Moving the guard (if self._content_error is not None: raise) to the top of the property avoids that dead retry. A regression test calling .content twice after a stream read failure, asserting the second call raises the stored exception without touching the raw stream, would cover it.