producer: Integrate context.Context with publishing

[Ulminator]

This PR sets out to resolve #360 with the integration of context.Context in the nsq.Producer. In order to maintain backwards compatibility, new methods were added that have names with the suffix WithContext. These new methods mostly correspond to those that a client would use to publish messages and they adhere to context timeouts/deadlines. Once the deadline has been reached, an error will be returned to the client and the given ProducerTransaction will be dropped/not published, all while keeping the connection to nsqd alive.

An overview of the new methods added are below:

nsq.Conn

• ConnectWithContext
• WriteCommandWithContext

nsq.Producer

• PingWithContext
• PublishWithContext
• MultiPublishWithContext
• DeferredPublishWithContext
• PublishAsyncWithContext
• MultiPublishAsyncWithContext
• DeferredPublishAsyncWithContext

The idea here being that these WithContext methods would essentially replace the existing ones eventually, which would be a breaking/major version change for go-nsq.

[mreiferson]

Thanks for this, a few minor comments. I don't have any better ideas on naming functions in backwards-compatible way.

[mreiferson]

I think we're shadowing err here and it won't be correct on line 323

[Ulminator]

Good catch! This should be fixed now 👍

[mreiferson]

I don't love this code we've copied in to this context-specific edge case, can't we consolidate this in to popTransaction? Do we need to define a context-specific error code for the protocol?

[Ulminator]

Yeah I wasn't a huge fan of how that was being handled...

I updated the code to invoke popTransaction without modifying the function signature, meaning I seemingly need to have at least one new "frameType" in protocol.go to handle the context errors. I opted to be explicit and add two "frameTypes" (for context.Cancelled and context.DeadlineExceeded respectively) so that we would not need to infer the context error from the byte slice/handle a default case like so:

if frameType == FrameTypeContextError {
    switch string(data) {
    case context.Canceled.Error():
        t.Error = context.Canceled
    case context.DeadlineExceeded.Error():
        t.Error = context.DeadlineExceeded
    default:
        t.Error = ???
    }
}

The FrameTypeContextCanceled and FrameTypeContextDeadlineExceeded are not really "frameTypes" per se, so I also don't fully love this approach, but it feels better than what it was.

I'm curious to hear your thoughts on this approach and any suggestions you may have on how to improve upon this.

[Ulminator]

@mreiferson thanks for the initial review!

Apologies it took so long for me to circle back around to this, but it should be good for another look 👍