Exit goroutine when stream closes

[n8maninger]

Cleans up the dangling goroutine if the passed in context.Context is not cancellable.

[lukechampine]

I don't love that this requires adding a field to the struct. :/ Isn't context.Background the only context that isn't cancelable?

[n8maninger]

Not ideal for sure. It bit me when debugging using context.Background. It’s probably unlikely in production, but an easy implementation detail to miss. Better to clean it up imo.

[lukechampine]

Honestly DialStreamContext probably shouldn't exist to begin with; retaining a passed-in context is a big red flag. Contexts are supposed to be scoped to the function they're passed to, whereas this function scopes the context to the Stream. Worse, the name is reminiscent of net.DialContext, where the context lets you cancel the dialing itself, which is not what DialStreamContext does at all. If the intention is to have a Stream that can be closed at any time by canceling a context, that's better accomplished like so:

func callRPCWithContext(ctx context.Context) error {
    s := mux.DialStream()
    defer s.Close()
    errChan := make(chan error)
    go func() { errChan <- doRPC(s) }()
    select {
    case <-ctx.Done():
        s.Close()
        <-errChan
        return ctx.Err()
    case err := <-errChan:
        return err
   }
}

You could even factor this out as a helper function:

func streamWithContext(ctx context.Context, s *mux.Stream, fn func(*mux.Stream) error) error {
    errChan := make(chan error)
    go func() { errChan <- fn(s) }()
    select {
    case <-ctx.Done():
        s.Close()
        <-errChan
        return ctx.Err()
    case err := <-errChan:
        return err
   }
}

This more accurately reflects how the lifetime of the context is tied to the lifetime of the stream.

[n8maninger]

Pushing it up the stack is also fine with me