gofiber
🐛 [Bug]: Adaptor does not flush when an net/http handler flushes
Bug Description
The current way the adaptor middleware works, is that it will complete the net/http handler first, then write to a fasthttp response object and return. This makes it incompatible with net/http handlers that may rely on instantly flushing to the client (e.g. Live Data Streams, SSE events).
(Sort of) related to issue #3386.
How to Reproduce
Steps to reproduce the behavior:
- Create a
net/httphandler
func handler(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
w.Header().Set("Cache-Control", "no-cache")
bw := bufio.NewWriter(w)
for i := 0; i < 10; i++ {
msg := fmt.Sprintf("Line %d\n", i)
if _, err := bw.WriteString(msg); err != nil {
fmt.Println("Error writing:", err)
return
}
if err = bw.Flush(); err != nil {
fmt.Println("Error flushing:", err)
return
}
if flusher, ok := w.(http.Flusher); ok {
flusher.Flush()
}
time.Sleep(1 * time.Second)
}
}
- Initialize a
fiberapp and use theadaptormiddleware to add a route to it.
app := fiber.New()
app.Get("/", adaptor.HTTPHandler(http.HandlerFunc(handler)))
app.Listen(":3000")
- Use curl to try to get a response.
curl localhost:3000
At this step, curl will block for 10 seconds before sending the full response, despite the Flush() calls used in the net/http handler.
Expected Behavior
The Adaptor middleware should have the response flush if the net/http handler defines such behavior. The behavior of w.Flush() and flusher.Flush() should mimic the usage of c.SendStreamWriter(streamWriter) if necessary to emit the correct response.
An issue with this is that it is hard to know when a net/http handler is flushing to the client, and treating all requests in this matter would incur a performance cost with flushing to the client on every write (Express does this behavior by default).
While I wouldn't recommend to do this for all requests since fasthttp tries to avoid flushing as much as it can, I would say it is worth having a way to instantly flush data if the net/http handler depends on this behavior.
Fiber Version
v3.0.0-beta.4
Code Snippet (optional)
package main
import (
"log"
"github.com/gofiber/fiber/v3"
"github.com/gofiber/fiber/v3/middleware/adaptor"
)
func main() {
app := fiber.New()
// Steps to reproduce
app.Get("/", adaptor.HTTPHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
w.Header().Set("Cache-Control", "no-cache")
bw := bufio.NewWriter(w)
defer bw.Flush()
for i := 0; ; i++ {
msg := fmt.Sprintf("Line %d\n", i)
_, err := bw.WriteString(msg)
if err != nil {
fmt.Println("Error writing:", err)
return
}
err = bw.Flush()
if err != nil {
fmt.Println("Error flushing:", err)
return
}
if flusher, ok := w.(http.Flusher); ok {
flusher.Flush()
}
time.Sleep(1 * time.Second)
}
}))
log.Fatal(app.Listen(":3000"))
}
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.
@grivera64 I notice this too, I don't think that's the use case of the adaptor. It's more for things that only support net/http and you want a quick(with overhead) way of accessing/exposing.
Probably worth investigating though
In my opinion, maybe the problem is caused by fasthttp.
package main
import (
"bufio"
"fmt"
"net/http"
"time"
"github.com/valyala/fasthttp"
"github.com/valyala/fasthttp/fasthttpadaptor"
)
func main() {
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
w.Header().Set("Cache-Control", "no-cache")
bw := bufio.NewWriter(w)
defer bw.Flush()
for i := 0; ; i++ {
msg := fmt.Sprintf("Line %d\n", i)
_, err := bw.WriteString(msg)
if err != nil {
fmt.Println("Error writing:", err)
return
}
err = bw.Flush()
if err != nil {
fmt.Println("Error flushing:", err)
return
}
if flusher, ok := w.(http.Flusher); ok {
flusher.Flush()
}
time.Sleep(1 * time.Second)
}
})
fastHTTPHandler := fasthttpadaptor.NewFastHTTPHandler(handler)
go http.ListenAndServe(":3001", handler)
fasthttp.ListenAndServe(":3000", fastHTTPHandler)
}
No response (fasthttp)
> curl localhost:3000
Response per seconds (net/http)
> curl localhost:3000
Line 0
Line 1
Line 2
Line 3
Line 4
Line 5
In my opinion, maybe the problem is caused by
fasthttp.
I submitted a PR to add flushing support for fasthttpadaptor in the fasthttp repo. Once merged, we can bump to the next release and close this issue.
@grivera64 We are waiting on 1.66.0 for this right?
@gaby Yes, after we address the failing workflow in #3741 with the upstream PR https://github.com/valyala/fasthttp/pull/2069, this should be fixed for the majority of cases.
The last case I see that could still have issues is when using a net/http handler as middleware. If a net/http middleware uses Flush() or Hijack(), then any other matched handlers currently would either:
- Fail to send a response
- Override the response
I think this limitation is similar to using SendStreamWriter() in a middleware in v3. All SendXXX() methods are mutually exclusive, and using several of these across several middleware has undefined behavior. (Their implementation within fasthttp will have one take precedence over the other and close the connection.)
Would you say that this is expected behavior rather than a bug for the middleware case?
@gaby Since #3790 was successfully merged and v1.67.0 has the added flushing functionality, should we now add unit tests to verify that the adaptor middleware flushing functionality is fully working?
There's a related issue with adaptor.FiberHandler, where it does not support buffered streams. Currently, it waits for the entire stream writer to finish writing, then sends the full response.
I can make a separate issue and submit a PR to fix it.