kennethreitz/requests — pull request #318 by atdt (2011-12-23 22:07:30+00:00)
Title: Make safe_mode handle exceptions more consistently

I'm not sure how I feel about this patch, but I thought I'd submit a pull request regardless because if nothing else it raises a couple of issues that need some attention, IMHO.

Instead of passing None to `_build_response` when a request failed in safe mode, we now pass an empty `urllib3.HTTPResponse` object instead. One advantage is that you no longer need to check for None in the `iter_*` methods, because reading the bare response works as expected (i.e., you end up with an empty iterator). Having a regular object is also nice because you can save suppressed exceptions in an attribute before passing it to `_build_response`, which allows us to avoid breaking the current pattern of having the nested build function there build a `requests.Response` by inspecting attributes of the raw response.

I took out the optional `is_error` arg in `_build_response` because it was not getting used anywhere, nor was it part of the public API.

But: is it a good idea to store suppressed exceptions in safe mode on `Response.error`? This makes `Response.ok` work as expected (or at least as *I* expect :P), but maybe this encourages bad habits. If you need to know what exceptions (if any) were raised by your HTTP request, you probably shouldn't be suppressing them.

On the other hand, the current implementation seems a little inconsistent:

```python
>>> import requests
>>> r = requests.get('http://__bad_url__/', config=dict(safe_mode=True))
>>> r.ok
True
>>> r = requests.get('http://httpbin.org/status/501', config=dict(safe_mode=True))
>>> r.ok
False
```

Thoughts?