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

New probe server fixes: allow accept_nonblock to raise and IO.gets return nil #1043

Merged
merged 2 commits into from
Aug 18, 2023
Merged

New probe server fixes: allow accept_nonblock to raise and IO.gets return nil #1043

merged 2 commits into from
Aug 18, 2023

Conversation

stas
Copy link
Contributor

@stas stas commented Aug 16, 2023

A couple of suggested fixes for the new probe http server. Commits should be self-explanatory, still some more context below.


Passing exceptions: false no longer raises IO::WaitReadable leaving the server in an unknown state.

By specifying a keyword argument exception to false, you can indicate that accept_nonblock should not raise an IO::WaitReadable exception, but return the symbol :wait_readable instead.


An IO.gets returns either a string or a nil. A nil response from a socket was not handled, suggesting we just skip those.


Appreciate taking a look as we're affected by these issues.

Passing `exceptions: false` no longer raises `IO::WaitReadable` leaving
the server in an unknown state.
@stas stas changed the title Let TCPServer raise exceptions. New probe server fixes Aug 16, 2023
@bensheldon
Copy link
Owner

@stas thank you! This change makes sense. I guess we were using exception: false but not checking if the return value == :wait_readable. From reading the docs, it wasn't entirely clear if there were other classes of exceptions that could be raised. But I'm ok accepting this and we'll see what we learn next 👍🏻

@bensheldon bensheldon merged commit b4fb7d6 into bensheldon:main Aug 18, 2023
19 checks passed
@bensheldon bensheldon added the bug Something isn't working label Aug 18, 2023
@bensheldon bensheldon changed the title New probe server fixes New probe server fixes: allow accept_nonblock to raise and IO.gets return nil Aug 18, 2023
@stas stas deleted the raise_exceptions_on_http_server_accept branch August 18, 2023 16:27
@stas stas restored the raise_exceptions_on_http_server_accept branch August 18, 2023 16:27
@bensheldon
Copy link
Owner

@stas I just released this in https://github.com/bensheldon/good_job/releases/tag/v3.17.3

@stas stas deleted the raise_exceptions_on_http_server_accept branch August 21, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

2 participants