Writing correct response writer wrappers

726 views
Skip to first unread message

Marco Jantke

unread,
Jan 16, 2018, 8:53:40 AM1/16/18
to golang-nuts
Hi everyone, 

I am currently working on a retry middleware for an http proxy. It is wrapping the original response writer and only in case the forwarded request should not be retried, it passes calls to `Write()`, `WriteHeader()` etc directly through to the original response writer. Now I am struggling with the fact, that response writers have different capabilities dependent on their type. Namely they can be `Hijacker`, `Flusher` or `CloseNotifier`. The problem is that my wrapper should have the same capabilities as the wrapped one, but I don't see any other way than creating all different types and using the proper one after doing type assertions on the response writer I am wrapping. Those types would be for example: `ResponseWriterWrapper`, `ResponseWriterWrapperWithFlush`, `ResponseWriterWrapperWithHijack`, `ResponseWriterWrapperWithHijackAndFlush`....


Does anyone have an idea how to approach this problem in a more reasonable fashion?

matthe...@gmail.com

unread,
Jan 16, 2018, 10:01:28 AM1/16/18
to golang-nuts
Can you provide some example code showing the problem?

I don’t understand why you are wrapping the ResponseWriter. If you need to pass along metadata then wrapping makes sense, but why not just pass the original interface and do the type switch in each handler?

type RetryResponseWriter struct {
    http
.ResponseWriter
   
Count uint
   

}

func handler1(w http.ResponseWriter, r *http.Request) {
    rrw
:= w.(RetryResponseWriter)
   
switch v := rrw.ResponseWriter.(type) {
   
case http.Hijacker:
       

   
case http.Flusher:
       

   
case http.CloseNotifier:
       

   
}
   

Matt

matthe...@gmail.com

unread,
Jan 16, 2018, 10:10:16 AM1/16/18
to golang-nuts
The type switch may not work since these http types are interfaces that may all be satisfied by the ResponseWriter. Instead of a type switch you would need to do a type assertion for each possible http interface.

Matt

Marco Jantke

unread,
Jan 17, 2018, 3:57:22 AM1/17/18
to golang-nuts
Thanks for the response Matt. Some examples to explain my use-case better. First of all, it's important to know that the proxy I am working on uses HTTP handler wrapper to provide additional functionality. The retry handler + response writer wrapper look like the following (code examples are simplified):


type retry struct {
  next *http.Handler
}

func (retry *Retry) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
attempts := 1
for {
retryResponseWriter := newRetryResponseWriter(rw, attempts >= retry.attempts, ...)

retry.next.ServeHTTP(retryResponseWriter, r.WithContext(newCtx))
if !retryResponseWriter.ShouldRetry() {
break
}

attempts++
log.Debugf("New attempt %d for request: %v", attempts, r.URL)
retry.listener.Retried(r, attempts)
}
}

type retryResponseWriter struct {
responseWriter    http.ResponseWriter
attemptsExhausted bool
  ...
}

// Only responses that should not be retried
// should be written into the original response writer.
func (rr *retryResponseWriter) ShouldRetry() bool {
return netErrorOccured() && !rr.attemptsExhausted
}

func (rr *retryResponseWriter) Header() http.Header {
if rr.ShouldRetry() {
return make(http.Header)
}
return rr.responseWriter.Header()
}

func (rr *retryResponseWriter) Write(buf []byte) (int, error) {
if rr.ShouldRetry() {
return 0, nil
}
return rr.responseWriter.Write(buf)
}

func (rr *retryResponseWriter) WriteHeader(code int) {
if rr.ShouldRetry() {
return
}
rr.responseWriter.WriteHeader(code)
}

func (rr *retryResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
return rr.responseWriter.(http.Hijacker).Hijack()
}

func (rr *retryResponseWriter) CloseNotify() <-chan bool {
return rr.responseWriter.(http.CloseNotifier).CloseNotify()
}

func (rr *retryResponseWriter) Flush() {
if flusher, ok := rr.responseWriter.(http.Flusher); ok {
flusher.Flush()
}
-}

My problem is now that the retryResponseWriter always implements the interfaces like Flusher or Hijacker. But actually what I want is that it has the same capabilities as the http.ResponseWriter I am wrapping. An idiomatic usage, to my understanding, of a response writer would look like the following:

if hijacker, ok := responseWriter.(Hijacker); ok {
  conn, rw, err := hijacker.Hijack()
  if err != nil {
    // error handling, I even saw sometimes panic() used in this cases
  }
  // handle connections
}

So implementing the interface is definitely something different than returning stub values. However, after having a closer look it looks like that it's safe to implement stub methods for Flush() (as a no-op) and CloseNotify() (returning a new closed channel). If that assumption is right I would only have to create two different retryResponseWriter types, one with Hijack and one without and do a type assertion on the http.ResponseWriter to choose which one. I would favor a more generic way in case someone has a better idea, though.

matthe...@gmail.com

unread,
Jan 17, 2018, 8:58:43 AM1/17/18
to golang-nuts
If you use struct embedding I think you may get what you are looking for:

type retryResponseWriter struct {
    http
.ResponseWriter
    attemptsExhausted
bool
}

Now retryResponseWriter has all of the methods of the http.ResponseWriter you assign to it and can be cast to those http interfaces without having to define stub methods or separate types.

Also since http.Handler is an interface I don’t think you need a pointer to it in type retry.

Matt
Reply all
Reply to author
Forward
0 new messages