py-amqp icon indicating copy to clipboard operation
py-amqp copied to clipboard

Potentially broken _AbstractTransport.__repr__

Open blueyed opened this issue 4 years ago • 3 comments

_AbstractTransport.__repr__ (or Connection.__repr__) might crash.

In a report/issue from/in Sentry self was reported as "broken repr" in https://github.com/celery/py-amqp/blob/7300741f9fc202083e87abd10e1cb38c28efad92/amqp/transport.py#L352.

_AbstractTransport.__repr__: https://github.com/celery/py-amqp/blob/7300741f9fc202083e87abd10e1cb38c28efad92/amqp/transport.py#L100-L106 Connection.__repr__: https://github.com/celery/py-amqp/blob/7300741f9fc202083e87abd10e1cb38c28efad92/amqp/connection.py#L281-L287

The full traceback:

ConnectionResetError: [Errno 104] Connection reset by peer
  File "celery/worker/loops.py", line 78, in asynloop
    update_qos()
  File "kombu/common.py", line 444, in update
    return self.set(self.value)
  File "kombu/common.py", line 437, in set
    self.callback(prefetch_count=new_value)
  File "celery/worker/consumer/tasks.py", line 43, in set_prefetch_count
    return c.task_consumer.qos(
  File "kombu/messaging.py", line 552, in qos
    return self.channel.basic_qos(prefetch_size,
  File "amqp/channel.py", line 1878, in basic_qos
    return self.send_method(
  File "amqp/abstract_channel.py", line 57, in send_method
    conn.frame_writer(1, self.channel_id, sig, args, content)
  File "amqp/method_framing.py", line 183, in write_frame
    write(view[:offset])
  File "amqp/transport.py", line 352, in write
    self._write(s)
BrokenPipeError: [Errno 32] Broken pipe
  File "kombu/message.py", line 128, in ack_log_error
    self.ack(multiple=multiple)
  File "kombu/message.py", line 123, in ack
    self.channel.basic_ack(self.delivery_tag, multiple=multiple)
  File "amqp/channel.py", line 1391, in basic_ack
    return self.send_method(
  File "amqp/abstract_channel.py", line 57, in send_method
    conn.frame_writer(1, self.channel_id, sig, args, content)
  File "amqp/method_framing.py", line 183, in write_frame
    write(view[:offset])
  File "amqp/transport.py", line 352, in write
    self._write(s)

I have not investigated why that is yet, but figured reporting it already is worth it, since having a proper (non-crashing) repr is more helpful during debugging/investigating issues, of course.

amqp 5.0.6 (but the code is the same in master) sentry-sdk 1.1.0

blueyed avatar May 19 '21 09:05 blueyed

Thank you @blueyed for your bug report. Unfortunately, I cannot understand what break in __repr__ from your report. I don't see anything regarding repr in your traceback. Could you be more specific?

matusvalo avatar May 20 '21 11:05 matusvalo

I have a similar problem. I don't yet understand why the socket is getting disconnected, but when it does the transport __repr__ throws an exception: [Errno 107] Transport endpoint is not connected in amqp.transport ine 119: https://github.com/celery/py-amqp/blob/86129b6f21ba3e3fca73c5dcc55256466c7c9a54/amqp/transport.py#L119

Because the socket is no currently connected, self.sock.getpeername() will raise an exception.

The __repr__ should not raise an exception here: socket errors are to be expected and attempting to log them shouldn't cause more errors.

One possible fix would be:

try:
    dst = f'{self.sock.getpeername()[0]}:{self.sock.getpeername()[1]}'
except (socket.error) as e:
    dst = f'ERROR: {e}'

nburlett avatar Apr 09 '24 17:04 nburlett