Mux.jl icon indicating copy to clipboard operation
Mux.jl copied to clipboard

Error on first request from Firefox

Open cmcaine opened this issue 4 years ago • 8 comments

The first time I visit a Mux server from firefox generally causes an error like this to be emitted by Mux. It seems to be harmless, but it would be nice to stop it.

I can't replicate the error with HTTP.get or Chrome.

┌ Error: error handling request
│   exception =
│    IOError: stream is closed or unusable
│    Stacktrace:
│      [1] check_open
│        @ ./stream.jl:386 [inlined]
│      [2] uv_write_async(s::Sockets.TCPSocket, p::Ptr{UInt8}, n::UInt64)
│        @ Base ./stream.jl:1018
│      [3] uv_write(s::Sockets.TCPSocket, p::Ptr{UInt8}, n::UInt64)
│        @ Base ./stream.jl:981
│      [4] unsafe_write(s::Sockets.TCPSocket, p::Ptr{UInt8}, n::UInt64)
│        @ Base ./stream.jl:1064
│      [5] unsafe_write
│        @ ~/.julia/packages/HTTP/IAI92/src/ConnectionPool.jl:171 [inlined]
│      [6] write
│        @ ./strings/io.jl:185 [inlined]
│      [7] write
│        @ ./io.jl:638 [inlined]
│      [8] unsafe_write(http::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.ConnectionPool.Transaction{Sockets.TCPSocket}}, p::Ptr{UInt8}, n::UInt64)
│        @ HTTP.Streams ~/.julia/packages/HTTP/IAI92/src/Streams.jl:100
│      [9] unsafe_write
│        @ ./io.jl:647 [inlined]
│     [10] write
│        @ ./io.jl:670 [inlined]
│     [11] handle(::HTTP.Handlers.RequestHandlerFunction{Mux.var"#7#8"{Mux.App}}, ::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.ConnectionPool.Transaction{Sockets.TCPSocket}})
│        @ HTTP.Handlers ~/.julia/packages/HTTP/IAI92/src/Handlers.jl:279
│     [12] #4
│        @ ~/.julia/packages/HTTP/IAI92/src/Handlers.jl:345 [inlined]
│     [13] macro expansion
│        @ ~/.julia/packages/HTTP/IAI92/src/Servers.jl:367 [inlined]
│     [14] (::HTTP.Servers.var"#13#14"{HTTP.Handlers.var"#4#5"{HTTP.Handlers.RequestHandlerFunction{Mux.var"#7#8"{Mux.App}}}, HTTP.ConnectionPool.Transaction{Sockets.TCPSocket}, HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.ConnectionPool.Transaction{Sockets.TCPSocket}}})()
│        @ HTTP.Servers ./task.jl:406
└ @ HTTP.Servers ~/.julia/packages/HTTP/IAI92/src/Servers.jl:373

cmcaine avatar Dec 20 '20 13:12 cmcaine

This is a common problem at the HTTP.jl level. There are numerous tickets there that have been outstanding for quite some time. Perhaps opening another ticket there with more detail might be useful? If you look at the Pluto.jl code base you'll see several hacks there that attempt to address this and other issues.

Oh. It seems at least one of these cases (https://github.com/JuliaWeb/HTTP.jl/pull/546) were plugged in HTTP v0.9.2 -- try again?

I was unable to test this because Mux seems to not permit newer versions of HTTP, as reported in https://github.com/JuliaWeb/Mux.jl/issues/132

clarkevans avatar Dec 23 '20 12:12 clarkevans

I still get this with HTTP.jl 0.9.2. You're right that this is an upstream issue, though.

cmcaine avatar Dec 27 '20 15:12 cmcaine

The HTTP.jl package has a decentralized approach for dealing with clients going away, each place where it can happen is its own if/then check. It'll be some significant refactor work to clean up these cases.

clarkevans avatar Dec 27 '20 16:12 clarkevans

Are you saying that just to comment (that's okay) or because you're expressing a preference that Mux should fix this at our level?

I opened an issue on HTTP.jl about where I think a possible fix is: https://github.com/JuliaWeb/HTTP.jl/issues/657

cmcaine avatar Dec 28 '20 09:12 cmcaine

It depends where the issue should be fixed. Jacob could argue that HTTP is a low-level library and should transparently propagate the error (in this case, he should remove the router logic before 1.0). In that circumstance, what's needed is a middleware layer that catches the errors and helps look for patterns that might help someone diagnosing network difficulties. Perhaps the best place for this middleware is in Mux.jl ?

clarkevans avatar Dec 28 '20 13:12 clarkevans

We currently use HTTP.serve, and with that these errors occur outside of the control of Mux, so we can't catch the errors.

The errors occur in the penultimate two lines of HTTP/src/Handlers.jl/handle():

"For request handlers, read a full request from a stream, pass to the handler, then write out the response"
function handle(h::RequestHandler, http::Stream, args...)
    request::Request = http.message
    request.body = read(http)
    closeread(http)
    request.response::Response = handle(h, request, args...)
    request.response.request = request
    startwrite(http)                               # <--- These two
    write(http, request.response.body)
    return
end

And everything that Mux does has already finished by then.

Obviously we could use the stream version of HTTP.serve and handle that ourselves, then it would be in our control.

cmcaine avatar Dec 29 '20 20:12 cmcaine

I think we need to see how we can get this addressed up stream. I also think that HTTP.jl should let the type of the response an IO object, detect this, and use Chunked-Encoding for delivery when the size of the IO is not known in advance. But rewriting an equivalent to Response seems like duplicative effort. Thanks for thinking about this.

clarkevans avatar Dec 29 '20 23:12 clarkevans

From the sideline: The code in WebSockets still carry traces from HttpServer.Server, as we felt we still needed somewhere to throttle the deeper stack traces and control REPL output. That doesn't really belong in WebSockets, and probably not in Mux or WebIO either?

hustf avatar Dec 30 '20 11:12 hustf

Closed as stale, following upstream.

cmcaine avatar Sep 06 '23 04:09 cmcaine