make skip IncompatibleProtocolError as optional for client to adapt scenarios like ELB connection

[jimhorng]

In my scenario with configurations

• tornado server publishes message to MQ through ELB
• ELB in front of rabbitMQ(s)
• rabbitMQ setup as a cluster

when some of rabbitMQ is down and the cluster is not in function, tornado's pika connection will receive error like below, and all the error handling and re-connection mechanism will NOT be invoked since the exception breaks the calls to _on_connection_closed which registered error handling callbacks.

"""
[2014-11-19 01:04:30,841][ERROR ][tornado.application:681] : Exception in I/O handler for fd 21#012Traceback (most recent call last):#1
2 File "/tornado/ioloop.py", line 671, in start#012 self._handlers[fd](fd,
events)#12 File "/tornado/stack_context.py", line 331, in wrapped#012 raise_exc_info(exc)#12 File "/tornado/stack_context.py", line 302, in wrapped#012
ret = fn(_args, *_kwargs)#12 File "/pika/adapters/base_connection.py”, line 322, in _handle_events#012 self._handle_read()#12
File "/pika/adapters/base_connection.py", line 348, in _handle_read#012
return self._handle_disconnect()#12 File "/pika/adapters/base_connection.py", line 248, in _handle_disconnect#012 self._adapter_disconnect()#12 File "/pika/adapters/tornado_connection.py", line 75, in _adapter_disconnect#012
super(TornadoConnection, self)._adapter_disconnect()#12 File "/pika/adapters/base_connection.py", line 137, in _adapter_disconnect#012
self._check_state_on_disconnect()#12 File "/pika/adapters/base_connection.py", line 149, in _check_state_on_disconnect#012
raise exceptions.IncompatibleProtocolError#012IncompatibleProtocolError
"""

/adapters/base_connection.py

    def _handle_disconnect(self):
        """Called internally when the socket is disconnected already
        """
        self._adapter_disconnect()   # BREAKS HERE !
        self._on_connection_closed(None, True)

In this scenario "IncompatibleProtocolError" is not treated as severe error and the connection can be recovered by re-connecting. Therefore I proposed that client can have option to skip this error for the appropriate scenario.

Thanks.

[coveralls]

Coverage increased (+3.3%) when pulling aa65fcb on jimhorng:feature/skip_IncompatibleProtocolError into 4429110 on pika:master.

[coveralls]

Coverage increased (+3.65%) when pulling 2d75088 on jimhorng:feature/skip_IncompatibleProtocolError into 4429110 on pika:master.

[gmr]

I'm not convinced this is the right way to handle this. IncompatibleProtocolError is raised when you open a connection and the remote side closes the connection before the AMQP connection can be negotiated. One could make a case that it should do an on_connection_error and that should contain the callback -- or that the TornadoConnection adapter some of Tornado's built in constructs to capture errors at this stage.

[jimhorng]

@gmr , thanks for comments,

1. do you mean adding another error capturer around
   pika/adapters/base_connection.py

def _handle_error(self, error_value):

for this kind of error? and set a flag to skip raising IncompatibleProtocolError
pika/adapters/base_connection.py

_check_state_on_disconnect()

so that self._on_connection_closed can be called to trigger connection.on_close() callback registered by client?

2. or create a new callback listener like on_connection_error around
   pika/adapters/base_connection.py

def _handle_error(self, error_value):

?

[gmr]

I think your problem may live with your ELB configuration. I use pika to talk to RabbitMQ through ELB and do not run into any issues. I am also not convinced the approach is correct.

I appreciate the PR but will not be merging it at this time. Thanks for your contribution and raising the issue.

[jimhorng]

@gmr , got it, thanks for taking time to look at this PR anyway.
I'l try to draft more solid scenario for reproducing this problem in future since I can reproduce it easily :(