using tusd handler with gorilla mux causing "NetworkControlError" warnings

[lukasrabener]

Question How to properly configure tusd handler to be used by gorilla mux without causing warnings of tusd middleware setting Read & Write Deadlines.

Setup details Please provide following details, if applicable to your situation:

• Operating System:Linux
• Used tusd version: 2.4.0
• Used tusd data storage: filestore
• Used tusd configuration: NotifyCompleteUploads: true;
• Used tus client library: -

when integrating the handler.NewHandler() into my existing gorilla mux with:

r.PathPrefix("/file/").Handler(http.StripPrefix("/file/", tusHandler))

i get alot of warnings for every request as the tusd middleware is trying to set Read & Write Deadlines which seems not supported:

WARN NetworkTimeoutError method=PATCH path=0d9b75efacca2786a9638d005c208b87 requestId="" id=0d9b75efacca2786a9638d005c208b87 error="feature not supported"

SetReadDeadline

[Acconut]

Can you provide a minimal example to reproduce this?

The code causing this warning uses http.NewResponseController, whose documentation hints at where the error might be coming from:

The ResponseWriter should be the original value passed to the [Handler.ServeHTTP] method, or have an Unwrap method returning the original ResponseWriter. [...] If the ResponseWriter does not support a method, ResponseController returns an error matching ErrNotSupported.

Could it be that mux only provides you with a wrapped ResponseWriter and not the original one? What version of HTTP does your requests have? HTTP/1.1, HTTP/2 or HTTP/3 over QUIC?

[lukasrabener]

hi, i adapted the basic example from the docs using gorilla mux, the warnings can be reproduced when using a middleware function with mux via r.Use().

example

```go package main import ( "fmt" "net/http" "time" "github.com/gorilla/mux" "github.com/tus/tusd/v2/pkg/filestore" tusd "github.com/tus/tusd/v2/pkg/handler" ) func main() { store := filestore.FileStore{ Path: "./uploads", } composer := tusd.NewStoreComposer() store.UseIn(composer) handler, err := tusd.NewHandler(tusd.Config{ BasePath: "/files/", StoreComposer: composer, NotifyCompleteUploads: true, }) if err != nil { panic(fmt.Errorf("unable to create handler: %s", err)) } go func() { for { event := <-handler.CompleteUploads fmt.Printf("Upload %s finished\n", event.Upload.ID) } }() r := mux.NewRouter() // mux middleware causing warnings r.Use(timeoutMiddleware(15 * time.Second)) r.PathPrefix("/files/").Handler(http.StripPrefix("/files/", handler)) srv := &http.Server{ Handler: r, Addr: "0.0.0.0:8080", WriteTimeout: 15 * time.Second, ReadTimeout: 15 * time.Second, } srv.ListenAndServe() } // example middleware func timeoutMiddleware(timeout time.Duration) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.TimeoutHandler(next, timeout, "timeout") } } ```

yes, i am getting a wrapped responsewriter from the middleware so that seems to cause the issue

[Acconut]

yes, i am getting a wrapped responsewriter from the middleware so that seems to cause the issue

In this case, the wrapped ResponseWriter should either implement an Unwrap method returning the original ResponseWriter, or it should implement the necessary control methods directly (SetReadDeadline and SetWriteDeadline in our case) as is explained at https://pkg.go.dev/net/http#NewResponseController.

If the the logic for wrapping the ResponseWriter is located in the mux package, you could open an issue in their repository asking if this is possible or whether there are other workarounds.

[lukasrabener]

adding an Unwrap() method to my custom ResponseController used in my middleware is fixing the issue, besides that the issue is still there when using the TimeoutHandler from go http package. It is just not having an Unwrap() method implemented, therefor it will trigger the mentioned warnings when using tusd handler.

thanks for reaching out, i will close this question

[Acconut]

the issue is still there when using the TimeoutHandler from go http package. It is just not having an Unwrap() method implemented, therefor it will trigger the mentioned warnings when using tusd handler.

I see, so this seems to be a shortcoming of Go's net/http package. I would recommend to report this to Go itself, so the necessary methods can be added to the TimeoutHandler's wrapped ResponseWriters.

[bfrederix]

@lukasrabener Can you share an example of how you implemented the Unwrap()?

[lukasrabener]

@lukasrabener Can you share an example of how you implemented the Unwrap()?

sure, it is just returning the original ResponseWriter from my custom one.

func (mcrw *myCustomResponseWriter) Unwrap() http.ResponseWriter {
    return mcrw.ResponseWriter
}

[bfrederix]

@lukasrabener Thank you. I did this exactly the same way, but Unwrap() didn't stop the log warnings for me. I needed to add SetWriteDeadline/SetReadDeadline to my custom ResponseWriter and middleware.

// ignoreNetworkWarnings is a required no-op because the datadog mux ResponseWriter does not support
// SetWriteDeadline and SetReadDeadline. This middleware ignores the SetWriteDeadline/SetReadDeadline warnings
// Read/Write timeouts are handled by the http.Server ReadTimeout and WriteTimeout config instead
type ignoreNetworkWarnings struct {
    http.ResponseWriter
}

func (mw *ignoreNetworkWarnings) SetWriteDeadline(t time.Time) error {
    return nil
}

func (mw *ignoreNetworkWarnings) SetReadDeadline(t time.Time) error {
    return nil
}

And I set these configurations on my server to hopefully replace what SetWriteDeadline/SetReadDeadline were doing.

// Create a new HTTP server using the mux router as the handler.
srv := &http.Server{
    Addr:              ":8080",
    ReadHeaderTimeout: 10 * time.Second,
    Handler:           muxTracer,
    BaseContext:       func(listener net.Listener) context.Context { return mainCtx },
    // ReadTimeout handles what SetReadDeadline would normally configure
    // Their normal default is 60 seconds, so we'll match that
    // https://github.com/tus/tusd/blob/15f05c021c9770dc0cd2818aa4ef44b9e361f5b2/pkg/handler/config.go#L97
    ReadTimeout: 60 * time.Second,
    // WriteTimeout handles what SetWriteDeadline would normally configure
    // Their normal default is 120 seconds, so we'll match that
    // https://github.com/tus/tusd/blob/15f05c021c9770dc0cd2818aa4ef44b9e361f5b2/pkg/handler/unrouted_handler.go#L181
    WriteTimeout: 120 * time.Second,
}

[Acconut]

@lukasrabener Please be aware that your implementation is not equivalent to what tusd does under the hood. With http.Server.ReadTime you specify the maximum duration that the server has to read the entire body. Effectively, you do not allow requests to be longer than 60 seconds. This might be OK if you ensure that all requests are below this threshold, but some uploads might take longer than this.

What tusd does is slightly different, while we are still receiving data from the client, we always extend the read deadline by 60 seconds. Meaning that a request does not have a fixed timeout, but instead times out 60 seconds after we received the last data. This allows PATCH requests to go on as long as they want, if they continuously upload data.

I did this exactly the same way, but Unwrap() didn't stop the log warnings for me.

Could it be that the ResponseWriter returned from Unwrap also needs to implement Unwrap?

[Acconut]

I just added some documentation regarding this issue: https://tus.github.io/tusd/advanced-topics/usage-package/#i-am-getting-warnings-regarding-networkcontrolerrornetworktimeouterror-and-feature-not-supported-why