🐛 [Bug]: Websocket middleware is not compatible with Recover middleware

wyyadd commented 1 year ago

Bug Description

Hi team! I have an issue. Websocket middleware is not compatible with Recover middleware.
If panic happens in websocket handler, process will quit.

Possible reason: In this code block, websocket middleware reigsters a handler to FastHTTPUpgrader.

        if err := upgrader.Upgrade(c.Context(), func(fconn *websocket.Conn) {
            conn.Conn = fconn
            defer releaseConn(conn)
            handler(conn)
        }); err != nil { // Upgrading required
            return fiber.ErrUpgradeRequired
        }

FastHTTPUpgrader also registers a HijackHandler.

func (u *FastHTTPUpgrader) Upgrade(ctx *fasthttp.RequestCtx, handler FastHTTPHandler) error {
        // ....
    ctx.Hijack(func(netConn net.Conn) {
                // ....
        handler(c)
                // ....
    })
        // ....
    return nil
}

FastHttp runs hijackConnHandler in a new goroutine. If a panic occurs within this goroutine, the Recover middleware is unable to catch it.

func (s *Server) serveConn(c net.Conn) (err error) {
        // ....
    if hijackHandler != nil {
                // ....
        go hijackConnHandler(ctx, hjr, c, s, hijackHandler)
                // ....
    }
        // ....
}

How to Reproduce

func main() {
    app := fiber.New()
    app.Use(recover.New(recover.Config{EnableStackTrace: true}))

    app.Get("/foo", websocket.New(func(c *websocket.Conn) {
        panic("foo")
    }))

    _ = app.Listen(":3000")
}

Expected Behavior

panic: foo
goroutine 20 [running]:
main.main.func1(0x6b8daf?)
        /home/leo/project/main.go:16 +0x27
github.com/gofiber/contrib/websocket.New.func2.5(0x0?)
        /home/leo/go/pkg/mod/github.com/gofiber/contrib/websocket@v1.0.0/websocket.go:130 +0x75
github.com/fasthttp/websocket.(*FastHTTPUpgrader).Upgrade.func1({0x9cf848, 0xc0000961e0})
        /home/leo/go/pkg/mod/github.com/fasthttp/websocket@v1.5.3/server_fasthttp.go:201 +0x1bf
github.com/valyala/fasthttp.hijackConnHandler(0x0?, {0x9cb180?, 0xc00009c060}, {0x9cf950, 0xc000090008}, 0xc00027e800, 0xc0000a2100)
        /home/leo/go/pkg/mod/github.com/valyala/fasthttp@v1.47.0/server.go:2505 +0x68
created by github.com/valyala/fasthttp.(*Server).serveConn
        /home/leo/go/pkg/mod/github.com/valyala/fasthttp@v1.47.0/server.go:2460 +0x1c1c

Contrib package Version

websocket v1.0.0

Code Snippet (optional)

package main

import "github.com/gofiber/contrib/%package%"

func main() {
  // Steps to reproduce
}

Checklist:

[X] I agree to follow Fiber's Code of Conduct.
[X] I have checked for existing issues that describe my problem prior to opening this one.
[X] I understand that improperly formatted bug reports may be closed without explanation.

baagod commented 1 year ago

What's the progress? Is no one paying attention to this issue?

ReneWerner87 commented 1 year ago

There are many topics in fiber and unfortunately not enough active developers who support these packages. We are depending on your help, because it is an open source project, which we are developing completely beside the rest of our daily tasks.

Our focus is on the core functionalities and the much used packages.

mstrYoda commented 1 year ago

/assign-to-me

oSethoum commented 1 year ago

@mstrYoda what's the status so far?

mstrYoda commented 1 year ago

@mstrYoda what's the status so far?

@efectn I think we can merge my pr that fixes this issue wdyt? 🤔

gofiber / contrib