Ensuring that the first argument to ConnectionError is a string

Putting an exception (or another type) as the first argument
to an Exception is confusing and makes it so that a caller's
"e.message" returns an Exception type rather than a string.

We spent quite a long time on this bug today -- a fix would be
much appreciated. Thanks!

--Robin


Thanks for raising this!

We actually quite like this behaviour, though: it means you can examine the exception object we wrapped. I'm going to let @sigmavirus24 weigh in here, but I think we're happy as-is.


Some of the exception objects that are nested inside of others at this point have much more debugging information than we can simply provide with a string. Further I have seen the underlying exception be something far more nefarious than what you're handling here: an exception caught and wrapped by another which was caught and wrapped by another which was caught and wrapped by another which was caught and wrapped by urllib3 which we then catch and wrap.

Granted I'm not sure we catch and reraise those consistently as `ConnectionError`s but I wouldn't be surprised if one snuck through like that.

On the other hand, I'm trying to imagine how you must have run into this and my own guess is that you did:

``` python
try:
    # something
except requests.exceptions.ConnectionError as e:
    log.debug(str(e))
```

And `str(e)` raised an exception that it was not returning a string as expected. Am I correct? If so a better fix, might be to change the behaviour of the exceptions a bit. It would allow old users relying on `e.message` to be an Exception class to have their cake and you to have yours as well.

If I'm wrong, please correct me so we can try to sort this out.


Well, we assumed that e.message would return a string. We've got a REST API that does certain things on certain exceptions, and then has a catch-all in case of any subclass of Exception. In that case, we log and then return as JSON the exception type and the exception message, using e.message.

Since we're turing the response into JSON, we expect e.message to be a string (which can be marshaled). In this case, it wasn't, which was confusing, broke our expectations, and as far as I understand (and please correct me if I'm wrong) broke a relatively strong Python convention, which is that e.message should (must?) be a string.

Thanks!

--Robin


It's only a convention because most of the time you can do `raise ValueError('message to print')`. There is nothing that says message must be a string though. It's also not that strong considering we receive errors that come from the standard library like that. That aside, what's wrong with calling `str(e.message)` to always be 100% certain it is marshal-able? If you're expecting a string you should make sure you coerce it to be safe.

If you were expecting it based on documentation of ours claiming it should be a string, I would be 100% supportive of you here, but we're not claiming that ever.

I've taken a similar approach in most of my projects (coercing) to be certain I'll have the API I'm expecting. It allows me to be far more confident in what I'm doing without having to worry about someone breaking from convention. It also allows me to have people pass in (for example) a string like `'123'` or just the number `123` since I'll always call `int()` on that input and if it isn't able to be coerced they will receive a helpful error from `int` that will be easier to Google.


@sigmavirus24 -- Thanks for the clarification. We were not relying on your documentation here; we were just leaning on past expectations of convention, which was clearly mistaken.

Thanks the for explanation, and you're right about coercing something into the format you expect it to be in, rather than just trying to marshal it and crossing your fingers.

Thanks the the replies!

--Robin


I'm glad we could help @hobbeswalsh 

Please continue sending PRs as you see fit! We don't mind discussing them with you. :)

Cheers! :wine_glass: