Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing response.content twice removes forgets read error #4965

Open
tlandschoff-scale opened this issue Feb 6, 2019 · 1 comment
Open
Labels

Comments

@tlandschoff-scale
Copy link

tlandschoff-scale commented Feb 6, 2019

I had a hard debugging time today because an error in the response stream is only reported when accessing response.content for the first time.

This is especially irritating when running code in a debugger.

Expected Result

If accessing response.content the first time raises an exception I would expect that accessing response.content again would also raise an exception (ideally the same).

Actual Result

Instead after raising on the first get, getting response.content again returns an empty string.

Reproduction Steps

Here is a patch with a new test case for this: error_replay_test.diff.gz.

Basically, it boils down to this:

import requests

response = requests.post("http://connreset.biz/get/incomplete/chunked", stream=True)
try:
    response.content
except Exception:
    # Error handling code, may try something else or fall through
    pass

content = response.content  # empty string

Output of my test case:

$ pipenv run py.test tests/test_lowlevel.py -q --tb=short -k retain
F                                                            [100%]
============================= FAILURES =============================
_______________ test_response_content_retains_error ________________
tests/test_lowlevel.py:343: in test_response_content_retains_error
    assert False, "error response has content: {0!r}".format(content)
E   AssertionError: error response has content: ''
E   assert False
1 failed, 15 deselected in 0.60 seconds

System Information

$ python -m requests.help

Edit: Oops, I used pipenv run python -m requests.help which actually called into system python 2.7. Here comes the real data:

$ pipenv run python3 -m requests.help
{
  "chardet": {
    "version": "3.0.4"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "2.7"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.6.8+"
  },
  "platform": {
    "release": "4.15.0-43-generic",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.21.0"
  },
  "system_ssl": {
    "version": "1000207f"
  },
  "urllib3": {
    "version": "1.24"
  },
  "using_pyopenssl": false
}

Thanks for looking into this!

@nateprewitt
Copy link
Member

It looks like we never closed the issue after the PR was merged. I'm reverting #5087 because it doesn't cover all of our read APIs (content, iter_content, iter_lines, text, __iter__) uniformly.

I took a quick pass but the edge cases the tests are missing aren't something I can get resolved today. We can reopen a new PR and revisit options later if anyone wants.

nateprewitt added a commit to nateprewitt/requests that referenced this issue Feb 18, 2020
…s read error."

This reverts commit bd10047.

This reverts commit d91fe00.
sethmlarson pushed a commit that referenced this issue Feb 19, 2020
…ead error."

This reverts commit bd10047.

This reverts commit d91fe00.
aless10 pushed a commit to aless10/requests that referenced this issue Feb 19, 2020
…s read error."

This reverts commit bd10047.

This reverts commit d91fe00.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants