Recovery middleware not working with Timeout middleware

[Vikash082]

I am using Timeout middleware with negroni . Sample code can be found here: https://gist.github.com/Vikash082/c3af4fb89728f816beb8204b4737d56d

But when there is panic occurs in my http handlers, negroni Recovery middleware doesn't work which means programs crashes. If I disable my Timeout middleware, program recovers as expected. Is there I need to change in my Timeout middleware to work with Recovery middleware.

[jszwedko]

Hi @Vikash082,

Unfortunately the Recovery middleware will not catch panics occurring in a separate goroutine (see #190 for more information).

I'm not super familiar with the context timeout pattern myself, but it seems like you should be able to do something like:

        ch := make(chan struct{})
        go func() {
                select {
                case <-ch:
                        return
                case <-ctx.Done():
                        w.WriteHeader(http.StatusRequestTimeout)
                        w.Write([]byte("Request Timed Out."))
                        return
                }
        }()

        next(w, r.WithContext(ctx))
        ch <- struct{}{}

Instead, to move the code likely to throw panics into the main request goroutine. You could also leave your code as-is, explicitly catch panics within the go func() {}, and reraise them outside.

Hope this helps!

[Vikash082]

@jszwedko Thanks for reply. Till time I was recovering from panic by using defer in below goroutine. So, which way (preferred) is better and I should follow ?

	go func() {
   		defer recovery(ch, w)
   		next(w, r.WithContext(ctx))
   		ch <- struct{}{}
   	}()
   	select {
   	case <-ch:
   		return
   	case <-ctx.Done():
   		msg := map[string]string{"err": "Request Timed Out"}
   		resp := utils.JsonResp(http.StatusRequestTimeout, msg)
   		resp.WriteHTTPResponse(w)
   		return
	   } 

[jszwedko]

Hi @Vikash082,

I would move next() out rather than adding an additional defer recovery, but that is just my preference. The only advantage I see to that is to just rely on the recovery middleware to catch the panics.

[Vikash082]

Hi @jszwedko

One issue which I see is, during Timeout. goroutine writes response, even the backend function after few second try to write response . In server we get print like this:

http: multiple response.WriteHeader calls

and client hangs indefinitely.

[jszwedko]

Ah, yes, I see. After looking more closely at net/http, it looks like r.WithContext is meant to be used when issuing HTTP requests rather than when responding to them.

It looks like there is a http.Handler wrapper that enforces a timeout, http.TimeoutHandler. It cuts execution short and writes a 503 instead of a 408, but you might be able to use the implementation as a reference for writing your own. As you can see, it actually runs the wrapped handler with a ResponseWriter to capture the output and only copies the headers and body if the handler did not time out. This avoids the issue with partially written requests that you ran into.

Hope this helps!

[jszwedko]

Closing since this is a known behavior per https://github.com/urfave/negroni/issues/190