Skip to content

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

Open
hellozzm wants to merge 1 commit into
psf:mainfrom
hellozzm:fix-response-content-error-retention
Open

Fix: retain stream read error on subsequent response.content access#7415
hellozzm wants to merge 1 commit into
psf:mainfrom
hellozzm:fix-response-content-error-retention

Conversation

@hellozzm

@hellozzm hellozzm commented May 6, 2026

Copy link
Copy Markdown

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'')

When accessing response.content raises an exception during stream reading,
subsequent accesses would silently return an empty bytestring instead of
re-raising the original error. This made debugging difficult, especially
in debuggers where properties may be accessed multiple times.

The fix stores the exception and re-raises it on subsequent accesses.

Fixes psf#4965

@golikovichev golikovichev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

2 participants