Reset the ProtocolBuffer transport socket upon a connection error

[bretthoerner]

Reset the ProtocolBuffer transport socket upon a connection error or broken pipe.

Related to #38

This ensures that a PB client object is not forever wedged after an attempt to connect fails, or after an existing connection is severed.

[reiddraper]

Based on comments here, it might just be socket.error, and no IOError too.

[bretthoerner]

Yeah, leaving the except open is pretty ugly but my thought was that socket.connect is so low-level and simple that if anything raises anything I sort of want to reset the socket.

I can change it, but I'm pretty sure nothing else could raise here that would make me want to not reset the socket.

[reiddraper]

I think if the socket should be reset on "anything going wrong", then it should be except: and not except Exception:.

I know this might seem pedantic, but do you have any reason to believe the state of the socket will be left invalid on anything but socket.error? I'm thinking about things like MemoryError.

[bretthoerner]

When I see except: I assume the person didn't think about what they are catching, when I see except Exception: I think "Ah, they really do mean anything (which I did).

Is MemoryError the only example? It is a little too pedantic to me. :) As a matter of fact, if connect is what raises a MemoryError then the only way it will currently be retried is if self._sock is set to None (so that it hits that branch on the next maybe_connect attempt).

All that said, I'm not afraid of MemoryError, I don't mind only catching socket.error. I'll change it tomorrow unless someone cares otherwise.

[reiddraper]

When I see except: I assume the person didn't think about what they are catching, when I see except Exception: I think
"Ah, they really do mean anything (which I did).

I see people use both of them wrong all the time. There is a difference between them, take a look at the hierarchy here. So, for example, if the program received KeyboardInterrupt during the socket connecting, but was then caught higher up in the call chain, the socket would not be set to None using except Exception. Try running this gist.

[bretthoerner]

Fair enough, I guess I'm really not worried about what happens when someone decides to C-c their process.

Also, I'm not sure which you're voting for now,

1. except: catches everything, including KeyboardInterrupt and destroys the socket so it retries next time, but is not specific as you asked for in your first comment.
2. except socket.error: only catches a socket problem, but things like MemoryError and KeyboardInterrupt will leave your socket in a wedged state that never attempts to reconnect again...

So... which is it. :)

[reiddraper]

Apologies for the runaround. I vote except:. If the client tried to reconnect, I'd vote except socket.error:.

[bretthoerner]

Fixed the except, any other thoughts?

[reiddraper]

Looks good to me :)