majek / puka

Puka - the opinionated RabbitMQ client
https://github.com/majek/puka
Other
182 stars 34 forks source link

Crash with an unhelpful error #35

Closed sgenoud closed 11 years ago

sgenoud commented 11 years ago

When I try to run the recieve.py example I get an unhelpful error:

Traceback (most recent call last):
  File "ex_puka.py", line 6, in <module>
    client.wait(promise)
  File ".../puka/puka/connection.py", line 233, in wait
    self.on_write()
  File ".../puka/puka/connection.py", line 193, in on_write
    r = self.sd.send(self.send_buf.read(128*1024))
socket.error: [Errno 32] Broken pipe

This is python 2.7, installed with brew on an OSX Mountain Lion machine. RabbitMQ runs on the standard port.

majek commented 11 years ago

Try with patch please, and let me know if it helps.

sgenoud commented 11 years ago

Now I get a new error linked with "read":

Traceback (most recent call last):
  File "ex_puka.py", line 8, in <module>
    client.wait(promise)
  File .../puka/puka/connection.py", line 236, in wait
    self.on_read()
  File ".../puka/puka/connection.py", line 96, in on_read
    r = self.sd.recv(131072)
socket.error: [Errno 61] Connection refused
majek commented 11 years ago

yes. Try replacing 'localhost' with '127.0.0.1' in the example

sgenoud commented 11 years ago

Thanks, it works! Can I ask what the problem is/was?

majek commented 11 years ago

puka tries ipv6 first, so your localhost was resolved as ::1 and for some reason rabbit doesn't listen on ipv6

schmir commented 11 years ago

isn't that enough reason to prefer IPv4 over IPv6? As discussed in #27 both choices are wrong, but to me it looks like one of them is "less wrong" :)

majek commented 11 years ago

@schmir for me it looks more like a bug in Rabbit, Rabbit should happily listen on ipv6 by default.

schmir commented 11 years ago

the people who try puka and run into problems because puka tries IPv6 first, probably just use another library. It doesn't matter to them if RabbitMQ should happily listen on IPv6.

Unless there's a good reason to use IPv6 first, please consider changing that!

majek commented 11 years ago

Okay, so what should Puka do?

schmir commented 11 years ago

I think it should iterate over all results from getaddrinfo until the first connection attempt succeeds (that's also what python's create_connection does). since the current non-blocking connection method complicates this, I would change it to a blocking connect if getaddrinfo returns more than one result, and use non-blocking connect if getaddrinfo returns exactly one result. in the former case the program already blocked in getaddrinfo resolving multiple addresses. in the latter case the caller probably just called it with an IP address and doesn't want the connection to block.

you've already had that discussion in #11

majek commented 11 years ago

you've already had that discussion in #11

You've started it here.

I think it should iterate over all results from getaddrinfo until the first connection attempt succeeds (that's also what python's create_connection does).

That's impossible with current API for Puka. With current design there is exactly one Client object for a socket descriptor. User can call client.fileno() at any time to get the descriptor number. That means - internally we have exactly one socket and we have exactly one chance to call connect() on it and try the ip address of the server. We can't "iterate over all results" unless we throw away the notion of a single socket for a Client object.

Alternatively we could make client.connect() blocking (it's non-blocking now) and make client.fileno() inaccessible before call to connect(). But that defeats the purpose of Puka IMO.

In other words: it's not that simple and we can't iterate over the results of getaddrinfo without making significant design changes.

schmir commented 11 years ago

Marek Majkowski notifications@github.com writes:

That's impossible with current API for Puka. With current design there is exactly one Client object for a socket descriptor. User can call client.fileno()` at any time to get the descriptor number. That means

  • internally we have exactly one socket and we have exactly one chance to call connect() on it and try the ip address of the server. We can't "iterate over all results" unless we throw away the notion of a single socket for a Client object.

It looks like I can call connect multiple times if the connection attempts fail (on linux). But as I said I probably wouldn't try several connects if they are nonblocking.

Alternatively we could make client.connect() blocking (it's non-blocking now) and make client.fileno() inaccessible before call to connect(). But that defeats the purpose of Puka IMO.

It's already blocking in getaddrinfo.

majek commented 11 years ago

It's already blocking in getaddrinfo.

Because there is no other choice. Fair enough, let's move the discussion to #36