1079515 - Informative error when login fails

Status: RESOLVED WORKSFORME

(Webtools :: Pulse, defect, P3)

Description

[Mark Côté [:mcote]]

When you try to connect to Pulse (or any RabbitMQ system) with invalid credentials, you get an "IOError: Socket closed" exception (sample traceback follows).  This is not particularly helpful.  Unfortunately, as mentioned, appears to be the default behaviour of kombu and/or amqp when construction a Connection object.  I'm not sure of the best way to solve this, nor if it needs to be solved in mozillapulse or in Pulse more generally (somehow), since I haven't used another AMQP client.

>>> c.listen()
---------------------------------------------------------------------------
IOError                                   Traceback (most recent call last)
<ipython-input-8-eb17c51d92c7> in <module>()
----> 1 c.listen()

/Users/mcote/projects/mozillapulse/src/mozillapulse/mozillapulse/consumers.pyc in listen(self, callback, on_connect_callback)
    147
    148         # Raise an error if the exchange doesn't exist.
--> 149         exchange(self.connection).declare(passive=True)
    150
    151         # Create a queue.

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/abstract.pyc in __call__(self, channel)
     64     def __call__(self, channel):
     65         """`self(channel) -> self.bind(channel)`"""
---> 66         return self.bind(channel)
     67
     68     def bind(self, channel):

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/abstract.pyc in bind(self, channel)
     68     def bind(self, channel):
     69         """Create copy of the instance that is bound to a channel."""
---> 70         return copy(self).maybe_bind(channel)
     71
     72     def maybe_bind(self, channel):

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/abstract.pyc in maybe_bind(self, channel)
     73         """Bind instance to channel if not already bound."""
     74         if not self.is_bound and channel:
---> 75             self._channel = maybe_channel(channel)
     76             self.when_bound()
     77             self._is_bound = True

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/connection.pyc in maybe_channel(channel)
   1052     otherwise just return the channel given."""
   1053     if isinstance(channel, Connection):
-> 1054         return channel.default_channel
   1055     return channel
   1056

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/connection.pyc in default_channel(self)
    754         """
    755         # make sure we're still connected, and if not refresh.
--> 756         self.connection
    757         if self._default_channel is None:
    758             self._default_channel = self.channel()

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/connection.pyc in connection(self)
    739                 self.declared_entities.clear()
    740                 self._default_channel = None
--> 741                 self._connection = self._establish_connection()
    742                 self._closed = False
    743             return self._connection

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/connection.pyc in _establish_connection(self)
    694     def _establish_connection(self):
    695         self._debug('establishing connection...')
--> 696         conn = self.transport.establish_connection()
    697         self._debug('connection established: %r', conn)
    698         return conn

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/kombu/transport/pyamqp.pyc in establish_connection(self)
    110             'heartbeat': conninfo.heartbeat,
    111         }, **conninfo.transport_options or {})
--> 112         conn = self.Connection(**opts)
    113         conn.client = self.client
    114         return conn

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/amqp/connection.pyc in __init__(self, host, userid, password, login_method, login_response, virtual_host, locale, client_properties, ssl, connect_timeout, channel_max, frame_max, heartbeat, on_blocked, on_unblocked, confirm_publish, **kwargs)
    178             self.wait(allowed_methods=[
    179                 (10, 20),  # secure
--> 180                 (10, 30),  # tune
    181             ])
    182

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/amqp/abstract_channel.pyc in wait(self, allowed_methods)
     65         default value of None means match any method), and dispatch to it."""
     66         method_sig, args, content = self.connection._wait_method(
---> 67             self.channel_id, allowed_methods)
     68
     69         return self.dispatch_method(method_sig, args, content)

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/amqp/connection.pyc in _wait_method(self, channel_id, allowed_methods)
    238         while 1:
    239             channel, method_sig, args, content = \
--> 240                 self.method_reader.read_method()
    241
    242             if channel == channel_id and (

/Users/mcote/projects/mozillapulse/lib/python2.7/site-packages/amqp/method_framing.pyc in read_method(self)
    187         m = self._quick_get()
    188         if isinstance(m, Exception):
--> 189             raise m
    190         if isinstance(m, tuple) and isinstance(m[1], AMQPError):
    191             raise m[1]

IOError: Socket closed

Comment 2

[Nick Alexander :nalexander [he/him]]

This is extremely confusing, because the various packages (mozillapulse, pulsebuildmonitor) suggest that they will work with user/password public/public and the documentation is not accurate.

It's even worse for pulsebuildmonitor because the failing listen() thread doesn't appear to surface its error.  I discovered this by re-implementing pieces of pbm until I discovered that mp was busted, and then trying to figure out why mp was busted.

Is it reasonable to just catch SocketErrors and annotate with a little "maybe you should check user/password?", at least as a stopgap?

Comment 3

[Mark Côté [:mcote]]

Well, SocketErrors can happen for all kinds of reasons, like simple, transitive network-connection problems... but perhaps we could log if we fail several times right away.  We should also fix the documentation.

Actually, what we *should* do is get a better mozillapulse library.  I believe jonasfj was planning to split out TaskCluster's Pulse Python library, which is more extensible, into a separate package for general consumption.  Rather than try to fix mozillapulse's problems, we should focus on using something that was better designed from the start (mozillapulse was always intended to be just a prototype anyway).  I filed bug 1133602 to track this.

Comment 4

[Ed Morley [:emorley]]

After filing:
https://github.com/rabbitmq/rabbitmq-server/issues/947

It turns out that rabbitmq supports a feature-bit that gives useful error messages when credentials are incorrect, however the client needs to support this:
http://www.rabbitmq.com/auth-notification.html

Kombu didn't support this, so I filed:
https://github.com/celery/kombu/issues/611

...and a fix for that is now included in py-amqp 2.1.1:
https://github.com/celery/py-amqp/commit/4d52fcee3320e19245818486c439e05b4a226f39

People should just update to py-amqp >= 2.1.1 - and given that the mozillapulse helper library doesn't pin the version of py-amqp, this is WFM :-)

Comment 6

[Mark Côté [:mcote]]

Awesome, thanks for following up on this!