Skip to content

Connections hijacked by ActionCable raise an error #150

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

Closed
yard opened this issue Mar 12, 2024 · 3 comments
Closed

Connections hijacked by ActionCable raise an error #150

yard opened this issue Mar 12, 2024 · 3 comments

Comments

@yard
Copy link
Contributor

yard commented Mar 12, 2024

When a connection gets hijacked by ActionCable, the HTTP1 server crashes the task – this seems to be a somewhat-known issue, has been mentioned in multiple places and indeed seems not-so-critical.

The culprit seems to be this line: https://github.com/socketry/async-http/blob/main/lib/async/http/protocol/http1/server.rb#L53. @stream is nil, by body is not (it's actually an instance of Protocol::Rack::Body::Enumerable). Looks like we can simply check for #empty? method on the body and get rid of the error.

I am happy to create a PR and submit it here, but before doing so, wanted to check if that is indeed a valid fix? Any implications of not sending the response when the body is empty and the @stream is nil? Or there is a case where this situation is valid and the fix belongs to a different place?

@ioquatix
Copy link
Member

Yes, I'm okay with the proposed PR. I may have additional feedback when I review it, but you are correct, that is the place to deal with it.

@yard
Copy link
Contributor Author

yard commented Mar 13, 2024

Thank you @ioquatix, I have opened a #151 that deals with it. Happy to iterate on it as per your feedback

@ioquatix
Copy link
Member

This should be fixed now, thanks to your effort. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants