Skip to content
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

proxy can error due to HTTP/2 stream resets #754

Closed
olix0r opened this issue Apr 12, 2018 · 8 comments
Closed

proxy can error due to HTTP/2 stream resets #754

olix0r opened this issue Apr 12, 2018 · 8 comments
Labels
priority/P0 Release Blocker
Milestone

Comments

@olix0r
Copy link
Member

olix0r commented Apr 12, 2018

As described in hyperium/h2#256, it's possible for some stream resets to cause h2 to panic. This is very likely to impact the proxy. @carllerche @seanmonstar

@olix0r olix0r added the priority/P0 Release Blocker label Apr 12, 2018
@olix0r olix0r added this to the 0.5.0 milestone Apr 12, 2018
@seeekr
Copy link

seeekr commented Apr 13, 2018

I'm seeing something like this:

conduit_proxy turning Error caused by underlying HTTP/2 error: protocol error: unexpected internal error encountered into 500

when I just terminate my HTTP server that conduit is proxying for, while processing a response and not having written anything to it yet.

Is this related? Sounds like it could be. Or should I file a separate issue for that?

@seanmonstar
Copy link
Contributor

@seeekr no, that would be different. A panic in the proxy will in most cases make it crash, not report a 500 error.

As to your issue... you're killing an HTTP server mid-response? Seems like the proxy seeing that and returning 500 is correct.

@seeekr
Copy link

seeekr commented Apr 13, 2018

Thanks @seanmonstar. If that's what it's doing then that is acceptable behavior. Just wasn't sure if this indicated a problem on conduit's part which I should look into understanding, since I was overall seeing some weird behavior in terms of connectivity for my service. (Killing it mid-response was not great, of course, and I've since changed that to be a little more graceful.)

Keep up the good work! 👍

@olix0r olix0r changed the title proxy can panic due to HTTP/2 stream resets proxy can error due to HTTP/2 stream resets Apr 16, 2018
@olix0r
Copy link
Member Author

olix0r commented Apr 16, 2018

hyperium/h2#258 will fix this. We learned that the conduit proxy will not panic in these situations, as it is built in release-mode. Panics are limited to debug builds.

@olix0r olix0r modified the milestones: 0.5.0, 0.4.1 Apr 16, 2018
@siggy
Copy link
Member

siggy commented Apr 25, 2018

Fixed in hyperium/h2#258. @olix0r please re-open if this is still an issue.

@siggy siggy closed this as completed Apr 25, 2018
@carllerche
Copy link
Contributor

I would leave this open for now. There still is one known issue. Relates to hyperium/h2#261 and hyperium/h2#259.

@carllerche carllerche reopened this Apr 25, 2018
@siggy siggy modified the milestones: 0.4.1, 0.5.0 Apr 25, 2018
@carllerche
Copy link
Contributor

I have squashed many bugs. There is still some work I would like to get done, then I will release a new crate.

Please ping me if this is not closed by next release.

carllerche added a commit that referenced this issue May 14, 2018
This required updating a few transitive dependencies to make the version
requirements work out.

Closes #754

Signed-off-by: Carl Lerche <me@carllerche.com>
@olix0r
Copy link
Member Author

olix0r commented May 14, 2018

h2-0.1.7 fixes a whole slew of state management corner-cases

@olix0r olix0r closed this as completed May 14, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority/P0 Release Blocker
Projects
None yet
Development

No branches or pull requests

5 participants