Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using tusd handler with gorilla mux causing "NetworkControlError" warnings #1100

Closed
lukasrabener opened this issue Mar 22, 2024 · 8 comments
Closed
Labels

Comments

@lukasrabener
Copy link

lukasrabener commented Mar 22, 2024

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
Copy link
Member

Acconut commented Mar 25, 2024

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
Copy link
Author

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
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
Copy link
Member

Acconut commented Mar 25, 2024

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
Copy link
Author

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
Copy link
Member

Acconut commented Apr 8, 2024

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
Copy link

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

@lukasrabener
Copy link
Author

lukasrabener commented Apr 12, 2024

@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
Copy link

bfrederix commented Apr 12, 2024

@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,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants