Feature: Return error if ResponseWriter does not implement Flush() for streaming RPCs.

[jchadwick-buf]

Is your feature request related to a problem? Please describe.
If you wrap your http.ResponseWriter in middleware before handing to connect go, you may wind up not exposing the http.Flusher interface, leading to calls to Send not actually flushing the http.ResponseWriter buffer and thus the connection appearing to "hang".

Describe the solution you'd like
This problem is tricky to detect and debug because it often works correctly; it only fails in some cases when using streaming RPCs. In an effort to make it harder to footgun oneself, I request that connect-go detect this situation and return an error from a streaming Send call.

Describe alternatives you've considered
Instead of returning an error, one could panic: this would make it much more noticeable. However, it is unclear if this is worse or better than unexpectedly hanging in production. I think returning an error that can be handled is at least obviously better. One could also make the case for some kind of warning message, but I'm not sure if there's a good way to do that (and perhaps, it's just not loud enough.)

Additional context
Apparently, the buf cli ran into this issue too, when migrating to Connect. It's a common footgun in Go. Maybe Go itself could be improved? I left some thoughts on golang/go#5465.

[akshayjshah]

Great idea! We can definitely make this better.

Maybe Go itself could be improved? I left some thoughts on golang/go#5465.

Re: your comments on this, it's unfortunately impossible to add new methods to http.ResponseWriter in Go 1. Any http.ResponseWriter implementations that don't currently implement http.Flusher will stop compiling, which breaks Go's back-compat guarantee.

[mattrobenolt]

Not a solution necessarily, but go1.20 is attempting to address this with their new http.ResponseController. https://pkg.go.dev/net/http@master#ResponseController

[jchadwick-buf]

@akshayjshah

Great idea! We can definitely make this better.

Maybe Go itself could be improved? I left some thoughts on golang/go#5465.

Re: your comments on this, it's unfortunately impossible to add new methods to http.ResponseWriter in Go 1. Any http.ResponseWriter implementations that don't currently implement http.Flusher will stop compiling, which breaks Go's back-compat guarantee.

Of course; that issue is the wishlist for backwards incompatible net/http changes in Go 2.

@mattrobenolt

Not a solution necessarily, but go1.20 is attempting to address this with their new http.ResponseController. https://pkg.go.dev/net/http@master#ResponseController

Nice. Maybe instead of my suggestion, it'd be better if they adopted Unwrap as part of the ResponseWriter interface instead.

[akshayjshah]

Of course; that issue is the wishlist for backwards incompatible net/http changes in Go 2.

🤦🏽‍♂️ I really ought to read more carefully (or avoid writing in the wee hours of the night). Mea culpa.