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

Static files are ending the connection with a protocol error with HTTP/2 #411

Closed
AaronFriel opened this Issue Jul 28, 2015 · 16 comments

Comments

Projects
None yet
5 participants
@AaronFriel
Contributor

AaronFriel commented Jul 28, 2015

I've got a Yesod front end running on a (now public) site.

Yesterday I was getting HTTP/2.0 protocol errors when accessing static files, e.g.: https://reactr.io/static/css/bootstrap.css

I restarted the site a few times, and it started working, so I moved on. I restarted it this morning after a user reported errors, and now those resources are failing (only over HTTP/2) reliably.

@creichert

This comment has been minimized.

Show comment
Hide comment
@creichert

creichert Jul 28, 2015

Member

@snoyberg I can't add a new label but I think it might be a good idea to add one for http2 to help @kazu-yamamoto and others organize them.

@AaronFriel Ideally, we can narrow this down to a minimal test case. If it ends up being a Warp bug it will be nice to add to the Spec.

Member

creichert commented Jul 28, 2015

@snoyberg I can't add a new label but I think it might be a good idea to add one for http2 to help @kazu-yamamoto and others organize them.

@AaronFriel Ideally, we can narrow this down to a minimal test case. If it ends up being a Warp bug it will be nice to add to the Spec.

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Jul 28, 2015

Member

Label created

On Tue, Jul 28, 2015, 12:48 PM Christopher Reichert <
notifications@github.com> wrote:

@snoyberg https://github.com/snoyberg I can't add a new label but I
think it might be a good idea to add one for http2 to help @kazu-yamamoto
https://github.com/kazu-yamamoto and others organize them.

@AaronFriel https://github.com/AaronFriel Ideally, we can narrow this
down to a minimal test case. If it ends up being a Warp bug it will be nice
to add to the Spec.


Reply to this email directly or view it on GitHub
#411 (comment).

Member

snoyberg commented Jul 28, 2015

Label created

On Tue, Jul 28, 2015, 12:48 PM Christopher Reichert <
notifications@github.com> wrote:

@snoyberg https://github.com/snoyberg I can't add a new label but I
think it might be a good idea to add one for http2 to help @kazu-yamamoto
https://github.com/kazu-yamamoto and others organize them.

@AaronFriel https://github.com/AaronFriel Ideally, we can narrow this
down to a minimal test case. If it ends up being a Warp bug it will be nice
to add to the Spec.


Reply to this email directly or view it on GitHub
#411 (comment).

@creichert creichert added the http2 label Jul 28, 2015

@AaronFriel

This comment has been minimized.

Show comment
Hide comment
@AaronFriel

AaronFriel Jul 28, 2015

Contributor

I can add that this fixes the problem, as opposed to using the Static site handlers.

getBootstrapR :: Handler TypedContent
getBootstrapR = return $ TypedContent typeCss
                    $ toContent $(embedFile "static/css/bootstrap.css")

I can also report that there may be some issue with sendFile.

Contributor

AaronFriel commented Jul 28, 2015

I can add that this fixes the problem, as opposed to using the Static site handlers.

getBootstrapR :: Handler TypedContent
getBootstrapR = return $ TypedContent typeCss
                    $ toContent $(embedFile "static/css/bootstrap.css")

I can also report that there may be some issue with sendFile.

@awpr

This comment has been minimized.

Show comment
Hide comment
@awpr

awpr Jul 28, 2015

Member

It would be informative to know what particular kind of protocol error it's reporting. https://github.com/yesodweb/wai/blob/master/warp/Network/Wai/Handler/Warp/HTTP2/Sender.hs#L54 is where (almost?) all protocol errors come from, so adding a trace of the exception there would probably pinpoint where the protocol error is being triggered.

Member

awpr commented Jul 28, 2015

It would be informative to know what particular kind of protocol error it's reporting. https://github.com/yesodweb/wai/blob/master/warp/Network/Wai/Handler/Warp/HTTP2/Sender.hs#L54 is where (almost?) all protocol errors come from, so adding a trace of the exception there would probably pinpoint where the protocol error is being triggered.

@awpr

This comment has been minimized.

Show comment
Hide comment
Member

awpr commented Jul 28, 2015

@AaronFriel

This comment has been minimized.

Show comment
Hide comment
@AaronFriel

AaronFriel Jul 28, 2015

Contributor

If there's no success replicating this tonight, I'll build my own Warp/WAI stack and add the traces. Just to minimize how deeply I have to grok the stack, could you tell me what to trace, and if I need any Show instances or the like?

Contributor

AaronFriel commented Jul 28, 2015

If there's no success replicating this tonight, I'll build my own Warp/WAI stack and add the traces. Just to minimize how deeply I have to grok the stack, could you tell me what to trace, and if I need any Show instances or the like?

@awpr

This comment has been minimized.

Show comment
Hide comment
@awpr

awpr Jul 28, 2015

Member

'err' and 'msg' both have Show instances that should give the necessary
info (err is the HTTP/2 error code and msg is a human-readable debug
message).

On Tue, Jul 28, 2015 at 3:00 PM Aaron Friel notifications@github.com
wrote:

If there's no success replicating this tonight, I'll build my own Warp/WAI
stack and add the traces. Just to minimize how deeply I have to grok the
stack, could you tell me what to trace, and if I need any Show instances or
the like?


Reply to this email directly or view it on GitHub
#411 (comment).

Member

awpr commented Jul 28, 2015

'err' and 'msg' both have Show instances that should give the necessary
info (err is the HTTP/2 error code and msg is a human-readable debug
message).

On Tue, Jul 28, 2015 at 3:00 PM Aaron Friel notifications@github.com
wrote:

If there's no success replicating this tonight, I'll build my own Warp/WAI
stack and add the traces. Just to minimize how deeply I have to grok the
stack, could you tell me what to trace, and if I need any Show instances or
the like?


Reply to this email directly or view it on GitHub
#411 (comment).

@awpr

This comment has been minimized.

Show comment
Hide comment
@awpr

awpr Jul 28, 2015

Member

I was able to reproduce this just by trying to send a file response and started digging around. I found this getting raised by fillResponseBodyGetNext and then ignored by the top level of the send loop:

Network/Wai/Handler/Warp/HTTP2/Sender.hs:245:9-34: Irrefutable pattern failed for pattern Just fdcache

For a workaround without code changes to Warp, try enabling the FD cache with Network.Wai.Handler.Warp.setFdCacheDuration.

Member

awpr commented Jul 28, 2015

I was able to reproduce this just by trying to send a file response and started digging around. I found this getting raised by fillResponseBodyGetNext and then ignored by the top level of the send loop:

Network/Wai/Handler/Warp/HTTP2/Sender.hs:245:9-34: Irrefutable pattern failed for pattern Just fdcache

For a workaround without code changes to Warp, try enabling the FD cache with Network.Wai.Handler.Warp.setFdCacheDuration.

@AaronFriel

This comment has been minimized.

Show comment
Hide comment
@AaronFriel

AaronFriel Jul 29, 2015

Contributor

Thanks for the quick discovery. Sounds like an easy fix for Warp, at least?

Contributor

AaronFriel commented Jul 29, 2015

Thanks for the quick discovery. Sounds like an easy fix for Warp, at least?

@AaronFriel

This comment has been minimized.

Show comment
Hide comment
@AaronFriel

AaronFriel Jul 29, 2015

Contributor

Site seems to be responding fine to static files now. For now, may I suggest changing the default FdCacheDuration and pushing a new version? Should keep anyone else from hitting this and thinking Warp HTTP/2 is broken.

Contributor

AaronFriel commented Jul 29, 2015

Site seems to be responding fine to static files now. For now, may I suggest changing the default FdCacheDuration and pushing a new version? Should keep anyone else from hitting this and thinking Warp HTTP/2 is broken.

kazu-yamamoto added a commit that referenced this issue Jul 29, 2015

@kazu-yamamoto

This comment has been minimized.

Show comment
Hide comment
@kazu-yamamoto

kazu-yamamoto Jul 29, 2015

Contributor

Sorry. This is my fault. The patch above would fix this issue.

However, file descriptors will leak. Does anyone have an idea to close file descriptors certainly?

Contributor

kazu-yamamoto commented Jul 29, 2015

Sorry. This is my fault. The patch above would fix this issue.

However, file descriptors will leak. Does anyone have an idea to close file descriptors certainly?

@AaronFriel

This comment has been minimized.

Show comment
Hide comment
@AaronFriel

AaronFriel Jul 29, 2015

Contributor

resourceT has awesome capabilities for managing contexts. It's not a heavy-weight dependency, but it would provide you with deterministic and garbage-collected control of closing file descriptors.

Contributor

AaronFriel commented Jul 29, 2015

resourceT has awesome capabilities for managing contexts. It's not a heavy-weight dependency, but it would provide you with deterministic and garbage-collected control of closing file descriptors.

@AaronFriel

This comment has been minimized.

Show comment
Hide comment
@AaronFriel

AaronFriel Jul 29, 2015

Contributor

To add to this example, here's what I've used:

In hyhac Database.HyperDex.Internal.Util.Resource I use a newtype around the InternalState to manage resources (in my cases, pointers to memory allocated by Haskell, and passed to a C API that is asynchronous.)

I don't do it yet, but I will use a ForeignPtr InternalState with a finalizer that calls closeInternalState to provide garbage-collected control of resources. That way, if my ResourceContext is garbage collected, I can mark the resources freed in an appropriate way.

With this ResourceContext, I wrote runInContext based on runResourceT. The difference: runInContext does not free resources when it is finished, but only on exceptions. (And in my cases, because the memory is volatile, I catch exceptional release conditions and leak memory, rather than free it and risk a use-after-free.

With runInContext, I can write an abstraction around these volatile C pointers that is safe, and is deterministic. I can call closeContext at any time.

Contributor

AaronFriel commented Jul 29, 2015

To add to this example, here's what I've used:

In hyhac Database.HyperDex.Internal.Util.Resource I use a newtype around the InternalState to manage resources (in my cases, pointers to memory allocated by Haskell, and passed to a C API that is asynchronous.)

I don't do it yet, but I will use a ForeignPtr InternalState with a finalizer that calls closeInternalState to provide garbage-collected control of resources. That way, if my ResourceContext is garbage collected, I can mark the resources freed in an appropriate way.

With this ResourceContext, I wrote runInContext based on runResourceT. The difference: runInContext does not free resources when it is finished, but only on exceptions. (And in my cases, because the memory is volatile, I catch exceptional release conditions and leak memory, rather than free it and risk a use-after-free.

With runInContext, I can write an abstraction around these volatile C pointers that is safe, and is deterministic. I can call closeContext at any time.

@kazu-yamamoto

This comment has been minimized.

Show comment
Hide comment
@kazu-yamamoto

kazu-yamamoto Jul 30, 2015

Contributor

I don't think that ResourceT fits this case. I would implement a designated thread to manage file descriptors with reaper.

Contributor

kazu-yamamoto commented Jul 30, 2015

I don't think that ResourceT fits this case. I would implement a designated thread to manage file descriptors with reaper.

@kazu-yamamoto

This comment has been minimized.

Show comment
Hide comment
@kazu-yamamoto

kazu-yamamoto Jul 30, 2015

Contributor

I noticed the the timer manager can release file descriptors.

Contributor

kazu-yamamoto commented Jul 30, 2015

I noticed the the timer manager can release file descriptors.

@kazu-yamamoto

This comment has been minimized.

Show comment
Hide comment
@kazu-yamamoto

kazu-yamamoto Jul 30, 2015

Contributor

Warp 3.1.1 has been released.

Contributor

kazu-yamamoto commented Jul 30, 2015

Warp 3.1.1 has been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment