-
-
Notifications
You must be signed in to change notification settings - Fork 150
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
huma panics on client context canceled #529
Comments
@nunoo do you mean these lines? https://github.com/danielgtaylor/huma/blob/main/huma.go#L487-L491. This happens after the handler has already returned the response struct, so there's no way to pass the error back to the handler. We could try to catch the context canceled scenario and set the response status appropriately to 499. How were you thinking this should be solved? |
This is our stack trace: /usr/local/go/src/runtime/panic.go:770 +0x132
github.com/danielgtaylor/huma/v2.transformAndWrite({0x15cbbf8, 0xc000459700}, {0x15df778, 0xc000345e00}, 0x1f3, {0xc000b70330, 0x10}, {0x12e3580, 0xc0009ca708})
/go/pkg/mod/github.com/danielgtaylor/huma/v2@v2.18.0/huma.go:483 +0x33b
github.com/danielgtaylor/huma/v2.Register[...].func1()
/go/pkg/mod/github.com/danielgtaylor/huma/v2@v2.18.0/huma.go:1349 +0x12b3
github.com/danielgtaylor/huma/v2/adapters/humago.(*goAdapter).Handle.func1({0x15c13e0, 0xc000d786a0}, 0xc000a8cc60)
/go/pkg/mod/github.com/danielgtaylor/huma/v2@v2.18.0/adapters/humago/humago.go:132 +0xa2
net/http.HandlerFunc.ServeHTTP(0x15c34a8?, {0x15c13e0?, 0xc000d786a0?}, 0x15a95c0?)
/usr/local/go/src/net/http/server.go:2171 +0x29 Although looking at the code, maybe we have this inverted, and there's a panic due to something else, which in turn cancels the context. |
@nunoo do you have a way to reproduce this issue? I can't seem to trigger the panic. I'v modified the greet example like this: package main
import (
"context"
"fmt"
"net/http"
"time"
"github.com/danielgtaylor/huma/v2"
"github.com/danielgtaylor/huma/v2/adapters/humago"
"github.com/danielgtaylor/huma/v2/humacli"
_ "github.com/danielgtaylor/huma/v2/formats/cbor"
)
// Options for the CLI.
type Options struct {
Port int `help:"Port to listen on" short:"p" default:"8888"`
}
// GreetingInput represents the greeting operation request.
type GreetingInput struct {
Name string `path:"name" maxLength:"30" example:"world" doc:"Name to greet"`
}
// GreetingOutput represents the greeting operation response.
type GreetingOutput struct {
Body struct {
Message string `json:"message" example:"Hello, world!" doc:"Greeting message"`
}
}
func main() {
// Create a CLI app which takes a port option.
cli := humacli.New(func(hooks humacli.Hooks, options *Options) {
// Create a new router & API
router := http.NewServeMux()
mw := func(h http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
start := time.Now()
h.ServeHTTP(w, r)
fmt.Printf("%s %s %s %s\n", r.Method, r.URL.Path, r.RemoteAddr, time.Since(start))
fmt.Println("middleware done")
})
}
api := humago.New(router, huma.DefaultConfig("My API", "1.0.0"))
// Register GET /greeting/{name}
huma.Register(api, huma.Operation{
OperationID: "get-greeting",
Summary: "Get a greeting",
Method: http.MethodGet,
Path: "/greeting/{name}",
}, func(ctx context.Context, input *GreetingInput) (*GreetingOutput, error) {
resp := &GreetingOutput{}
resp.Body.Message = fmt.Sprintf("Hello, %s!", input.Name)
time.Sleep(5 * time.Second)
return resp, nil
})
// Tell the CLI how to start your router.
hooks.OnStart(func() {
http.ListenAndServe(fmt.Sprintf(":%d", options.Port), mw(router))
})
})
// Run the CLI. When passed no commands, it starts the server.
cli.Run()
} Then I run it, and use restish or curl to hit the URL (e.g. |
@danielgtaylor thanks for taking a deeper look into this! The error I was seeing may have been related to the fix in #542 so I'll wait until that is released to see if the issue persists |
On line
huma.go:483
, if the client cancels the context, it results in a panic. This explicitly removes control flow from the user, and actually ends up writing out a status 500 in any lower level middlewares (like an http.Handler middleware receives a 500, instead of the 499 it should've been, had we propagated the context canceled error)The text was updated successfully, but these errors were encountered: