-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
HTTP::Client spec: don't write server response if expecting read timeout #7402
HTTP::Client spec: don't write server response if expecting read timeout #7402
Conversation
That's what I would expect at least.. |
Ugh, that failure in darwin... come on, CI, I'm trying my best here, don't give me new errors... |
For history purposes, this is the darwin build that failed: https://circleci.com/gh/crystal-lang/crystal/18231?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link I'll re-run it now. |
054a455
to
7725449
Compare
I'm pretty sure the immediate issue should be fixed by #7197. |
Another one to investigate, a |
That's already tracked in #7243 |
We can probably merge this PR first, which is really small, and then focus on reviewing #7197, which is much larger and also is currently failing on my machine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this would really be required to keep in the long run, but it's fine by me.
Fixes #7401
I found out that the problem was in the "tests read_timeout" spec. This fixes it but I'm not entirely sure why the exception happens.
What I think happens is:
At least I couldn't reproduce it with
GC_DONT_GC=1
(but then it failed because of too many open file requests), so I'm not sure. But I think my logic above is correct.In this PR, when we expect a read timeout to happen we don't even bother instructing the server to write a response because that doesn't matter anymore.
However, this leads me to think... if I write an HTTP server and I accept a request and the client closes the socket before I get to write the response... an exception will be raised and it will be printed in the server's
spawn
handling that request. Is that OK? How do other languages handle this?