Unhandled error, publishing to an exchange using an invalid exchange type

[cpg1111]

Hi there,
I seemed to have been getting panic when writing to a closed (go not rabbit) channel, which completely makes sense. However, I got in this state due to a typo in my exchange type, x-consistent-hash was spelled x-constistent-hash. I think I see a couple ways this error state can be handled gracefully, if this change is welcomed, I'd be more than happy to create a pull request for this. Returning an error for this on declare of the exchange seems to make the most sense to me, but I'm certainly open to discussing.

[cpg1111]

This #196, seems related to what happens when this isn't caught.

[michaelklishin]

Specifying an incorrect exchange type will result in an unrecoverable (connection) exception. What kind of improvements do you have in mind?

[michaelklishin]

#196 does look relevant but I don't remember what the decision was (if any). Maybe @gerhard or @streadway do :)

[cpg1111]

I'm certainly open to suggestions if this is not ideal, but you could validate the exchange type, possibly using a map[string]bool, where the keys are valid exchange types, though I can understand if that's not ideal due to the need to keep it up to date every time valid exchange types are either removed or added.

[michaelklishin]

Exchange types can be provided by plugins. We cannot limit types to the list of built-in or known ones.

[cpg1111]

figured so, hmm, I'll have to look into what happens when declaring an exchange of a wrong type a little further then.

[michaelklishin]

@cpg1111 it would be much easier to suggest something if you could post your code. The issue is more about connection.close handling than exchanges or exchange types. In fact, I wouldn't be surprised if it's a different manifestation of #196.

[cpg1111]

Yes it definitely is #196 that causes the panic, tracing my code shows that. My issue is more about handling my cause of that beforehand. I am saying handle the wrong exchange type, not the race condition (in this issue specifically). Here's my code:

for <condition arbitrary> {
    go func(ex, exType string, c *amqp.Conn){
        var err error
        channel := c.Channel()
        err = channel.ExchangeDeclare(ex, exType, true, false, false, false, nil)
        if err != nil {
           log.Error(err)
           return
        }
        err = channel.Publish(ex, exT, "comeFromConfig",  true, false, amqp.Publishing{
            ContentType: "someMSGType",
            ContentEncoding: "utf-8",
            DeliveryMode: 2,
            Body: <some body>,
        })
        if err != nil {
             log.Error(err)
        }
    }("exchange", "x-consistent-hash", conn) // originally mispelled --> "x-constistent-hash"
}

[michaelklishin]

@cpg1111 how can the library know what exchange type is "wrong" if it can be any string that begins with an x-?

Some other clients, e.g. Bunny, have helpers for known exchange types: topic, fanout, direct, etc.
We could introduce additional functions that declare exchanges of that type but I have a feeling @streadway won't be happy about the explosion of functions with more or less identical purposes.

Another thing we can do is to introduce constants for known exchange types. I think this should be an easier sell.

[cpg1111]

I do like the constants idea personally. I was thinking of handling a failed attempt after it's sent to rabbitmq in Channel's sendOpen function, but after further inspection, I see you won't get an explicit error from rabbit of a bad exchange type.

[michaelklishin]

@cpg1111 constants is an easy and no risk solution. @gustamora sorry, can you elaborate?

[michaelklishin]

@cpg1111 we believe registering an error handler on the connection is what you want. @streadway @gerhard and I agree that introducing constants for known exchange types is a good idea.

[cpg1111]

@michaelklishin yeah I've come to that inclusion, thanks for pointing me there. And yeah I think the constants would would be a great idea.