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

[question] about panic with error marshaling response: ... context canceled. #632

Open
superstas opened this issue Oct 31, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@superstas
Copy link
Contributor

Hi there,

I have a question about the panic that happens here.

We've been facing such panic in our logs for some time, and the err is context canceled.

We use humaecho adapter that uses echo.Context.Response under the hood.

Sometimes, we get context canceled err on api.Marshal(ctx.BodyWriter(), ct, tval), which forces Huma to panic.

How to reproduce?

I took humamux adapter and added mockBodyWriter that returns context.Canceled

mock_adapter.go

package main

import (
	"context"
	"crypto/tls"
	"io"
	"mime/multipart"
	"net/http"
	"net/url"
	"time"

	"github.com/danielgtaylor/huma/v2"
	"github.com/danielgtaylor/huma/v2/queryparam"
	"github.com/gorilla/mux"
)

// mockBodyWriter is a mock implementation of io.Writer that returns context.Canceled error.
type mockBodyWriter struct{}

func (w mockBodyWriter) Write(p []byte) (n int, err error) {
	return 0, context.Canceled
}

// MultipartMaxMemory is the maximum memory to use when parsing multipart
// form data.
var MultipartMaxMemory int64 = 8 * 1024

type gmuxContext struct {
	op     *huma.Operation
	r      *http.Request
	w      http.ResponseWriter
	status int
}

// check that gmuxContext implements huma.Context
var _ huma.Context = &gmuxContext{}

func (c *gmuxContext) Operation() *huma.Operation {
	return c.op
}

func (c *gmuxContext) Context() context.Context {
	return c.r.Context()
}

func (c *gmuxContext) Method() string {
	return c.r.Method
}

func (c *gmuxContext) Host() string {
	return c.r.Host
}

func (c *gmuxContext) RemoteAddr() string {
	return c.r.RemoteAddr
}

func (c *gmuxContext) URL() url.URL {
	return *c.r.URL
}

func (c *gmuxContext) Param(name string) string {
	return mux.Vars(c.r)[name]
}

func (c *gmuxContext) Query(name string) string {
	return queryparam.Get(c.r.URL.RawQuery, name)
}

func (c *gmuxContext) Header(name string) string {
	return c.r.Header.Get(name)
}

func (c *gmuxContext) TLS() *tls.ConnectionState {
	return c.r.TLS
}

func (c *gmuxContext) Version() huma.ProtoVersion {
	return huma.ProtoVersion{
		Proto:      c.r.Proto,
		ProtoMajor: c.r.ProtoMajor,
		ProtoMinor: c.r.ProtoMinor,
	}
}

func (c *gmuxContext) EachHeader(cb func(name, value string)) {
	for name, values := range c.r.Header {
		for _, value := range values {
			cb(name, value)
		}
	}
}

func (c *gmuxContext) BodyReader() io.Reader {
	return c.r.Body
}

func (c *gmuxContext) GetMultipartForm() (*multipart.Form, error) {
	err := c.r.ParseMultipartForm(MultipartMaxMemory)
	return c.r.MultipartForm, err
}

func (c *gmuxContext) SetReadDeadline(deadline time.Time) error {
	return huma.SetReadDeadline(c.w, deadline)
}

func (c *gmuxContext) SetStatus(code int) {
	c.status = code
	c.w.WriteHeader(code)
}

func (c *gmuxContext) Status() int {
	return c.status
}

func (c *gmuxContext) AppendHeader(name string, value string) {
	c.w.Header().Add(name, value)
}

func (c *gmuxContext) SetHeader(name string, value string) {
	c.w.Header().Set(name, value)
}

func (c *gmuxContext) BodyWriter() io.Writer {
	return mockBodyWriter{}
}

type gMux struct {
	router *mux.Router
}

func (a *gMux) Handle(op *huma.Operation, handler func(huma.Context)) {
	a.router.
		NewRoute().
		Path(op.Path).
		Methods(op.Method).
		HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			handler(&gmuxContext{op: op, r: r, w: w})
		})
}

func (a *gMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	a.router.ServeHTTP(w, r)
}

func New(r *mux.Router, config huma.Config) huma.API {
	return huma.NewAPI(config, &gMux{router: r})
}

main.go

package main

import (
	"context"
	"net/http"

	"github.com/danielgtaylor/huma/v2"
	"github.com/gorilla/mux"
)

type (
	User struct {
		Name    string `json:"name" minLength:"1" maxLength:"32"`
		Surname string `json:"surname" minLength:"1" maxLength:"32"`
	}

	CreateUserInput struct {
		Body User
	}

	CreateUserOutput struct {
		Body struct {
			Message string `json:"message"`
		}
	}
)

func addRoutes(api huma.API) {
	huma.Register(api, huma.Operation{
		OperationID: "CreateUser",
		Method:      http.MethodPost,
		Path:        "/user",
	}, func(ctx context.Context, input *CreateUserInput) (*CreateUserOutput, error) {
		resp := &CreateUserOutput{}
		resp.Body.Message = "CreateUser works!"
		return resp, nil
	})
}

func main() {
	router := mux.NewRouter()
	api := New(router, huma.DefaultConfig("My API", "1.0.0"))
	addRoutes(api)

	http.ListenAndServe("127.0.0.1:8888", router)
}

cURL request

$> curl -v http://127.0.0.1:8888/user -H "Content-Type: application/json" -d '{"name": "Foo", "surname": "Bar"}'

*   Trying 127.0.0.1:8888...
* Connected to 127.0.0.1 (127.0.0.1) port 8888 (#0)
> POST /user HTTP/1.1
> Host: 127.0.0.1:8888
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 33
> 
* Empty reply from server
* Closing connection 0
curl: (52) Empty reply from server

Server output

2024/10/31 13:44:52 http: panic serving 127.0.0.1:35094: error marshaling response for POST /user 200: context canceled

goroutine 20 [running]:
net/http.(*conn).serve.func1()
	/home/superstas/.gvm/gos/go1.23.2/src/net/http/server.go:1947 +0xbe
panic({0x719be0?, 0xc000140c00?})
	/home/superstas/.gvm/gos/go1.23.2/src/runtime/panic.go:785 +0x132
github.com/danielgtaylor/huma/v2.transformAndWrite({0x7fd0f8, 0xc0001880e0}, {0x7fe3b8, 0xc0001d6d20}, 0xc8, {0x76dc86, 0x10}, {0x71c9e0, 0xc00011fd80})
	/home/superstas/.gvm/pkgsets/go1.23.2/global/pkg/mod/github.com/danielgtaylor/huma/[email protected]/huma.go:480 +0x313
github.com/danielgtaylor/huma/v2.Register[...].func1()
	/home/superstas/.gvm/pkgsets/go1.23.2/global/pkg/mod/github.com/danielgtaylor/huma/[email protected]/huma.go:1465 +0x105c
main.(*gMux).Handle.func1({0x7fb1e0, 0xc000188380}, 0xc0001fe280)
	/home/superstas/projects/humatest/adapter_mock.go:138 +0xa2
net/http.HandlerFunc.ServeHTTP(0xc0001fe140?, {0x7fb1e0?, 0xc000188380?}, 0x10?)
	/home/superstas/.gvm/gos/go1.23.2/src/net/http/server.go:2220 +0x29
github.com/gorilla/mux.(*Router).ServeHTTP(0xc0001c60c0, {0x7fb1e0, 0xc000188380}, 0xc0001fe000)
	/home/superstas/.gvm/pkgsets/go1.23.2/global/pkg/mod/github.com/gorilla/[email protected]/mux.go:212 +0x1e2
net/http.serverHandler.ServeHTTP({0x7f9f28?}, {0x7fb1e0?, 0xc000188380?}, 0x6?)
	/home/superstas/.gvm/gos/go1.23.2/src/net/http/server.go:3210 +0x8e
net/http.(*conn).serve(0xc0001902d0, {0x7fb648, 0xc0001d6a50})
	/home/superstas/.gvm/gos/go1.23.2/src/net/http/server.go:2092 +0x5d0
created by net/http.(*Server).Serve in goroutine 1
	/home/superstas/.gvm/gos/go1.23.2/src/net/http/server.go:3360 +0x485

Questions

  • First, I'm trying to understand why Huma is panicking here. Since this use case looks kind of "legit" ( for example, a client is gone ), recovered panics ( with this error ) look like false-positive alerts for us at the moment.
  • What other alternatives are out there for such cases? When Huma can't marshal/deliver a response to the client, there is not much we can do, but panicking still doesn't look like the best option, to be honest.

Overall, I want to understand the reasons behind this logic before doing anything else.

Thank you.

@superstas superstas changed the title [question] about error marshaling response: ... context canceled. [question] about panic with error marshaling response: ... context canceled. Oct 31, 2024
@danielgtaylor
Copy link
Owner

@superstas thanks for the question. The client disconnecting is indeed a valid use case. We have seen these messages too and tend to ignore them, but it's probably better to filter these panics out in Huma now that I'm thinking about it. We could check if marshaling fails and if the context is canceled we just stop. Something like this:

if ctx.Context().Err() == context.Canceled {
	// The client disconnected, so don't bother writing anything.
	return
}

@danielgtaylor danielgtaylor added the bug Something isn't working label Nov 7, 2024
@superstas
Copy link
Contributor Author

@danielgtaylor thank you for the reply.

Yes, checking the root cause of the marshaling failure and filtering out disconnected client cases sounds logical to me as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants