Catching socket.error raises `ValueError`

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Invoke any method that revolves around sending/receiving data through a 
socket (`send_message`, `read_message`, `send_action`, etc.) in ami.py
2. Wait for or force a failure in the connection that will result in a 
socket.error exception being raised. Every time, a line like this will try to 
catch it:

    ...
    except socket.error as (errno, message):

3. If either errno or message is `None` that line will raise a `ValueError` and 
the exception will not be handled, sometimes failing to close the connection 
(for instance, line 1111 and 1112 in `read_message` of ami.py)

What is the expected output? What do you see instead?

socket.error exception should raise a new `ManagerSocketError` that would allow 
the client to reconnect or shutdown gracefully. Instead, `ValueError` is 
raised, the connection is not closed and `is_connected`  still returns `True`.

What version of the product are you using? On what operating system?

pystrix trunk on Ubuntu 13.10 (python 2.7)

Please provide any additional information 
below.

A simple change to the way socket.error is caught should suffice:

      except socket.error as e:
                    self._close()
                    raise ManagerSocketError("Connection to Asterisk manager broken while reading data: [{errno}] {error}"
            .format(errno=e.errno, error=e.strerror))

Original issue reported on code.google.com by nfantone on 10 Jan 2014 at 2:18

[GoogleCodeExporter]

Thanks for reporting this, nfantone; I'll need to make some changes to other 
projects where I forgot that socket.error could omit errno, too. There are also 
three other places in pystrix where this is a relevant change.

As much as I love namedtuples and am starting to appreciate .format(), the 
solution you proposed isn't backwards-compatible (and it would generate an ugly 
[None] in the explanation-text), so I'm using a slightly less-efficient 
approach in my fix.

The patch should be committed to version-control by the time you see this, and 
pystrix is scheduled to get some of my time in February to keep pushing its 
documentation and Asterisk-bindings towards the 1.10 and 1.11 releases I've 
mentioned before.

Original comment by red.hamsterx on 10 Jan 2014 at 5:19

• Changed state: Started
• Added labels: Priority-Critical
• Removed labels: Priority-Medium

[GoogleCodeExporter]

You are welcome. Glad to give something back.

Original comment by nfantone on 13 Jan 2014 at 6:56