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

Make sleep_on_error timeout configurable, enable it by default + document it #1455

Closed
klausi opened this issue Mar 3, 2018 · 3 comments
Labels

Comments

@klausi
Copy link
Contributor

@klausi klausi commented Mar 3, 2018

Follow-up to #1358 .

Now that we have internal IO error handling when a Hyper server is under heavy load (or DoS attack) we should make this configurable and enabled by default for a better developer experience.

@BatmanAoD

This comment has been minimized.

Copy link

@BatmanAoD BatmanAoD commented Mar 16, 2018

Disclaimer: I am not a web developer, either frontend or backend.

Swallowing errors by default seems like a terrible idea to me. This is the best way to make unreliable and difficult to debug software. In fact, JavaScript's "swallow errors by default" design philosophy is perhaps the primary reason why I'm not a web developer; I was hoping that a full-stack Rust framework (with WebAssembly on the frontend) would make the idea more appealing to me, because Rust seems to have precisely the opposite design philosophy.

However, it does seem that for a DoS attack or heavy load, the server needs to be able to ignore requests for some short period of time. But the user should still be able to do something with the specific errors that are generated.

So my suggestion would be to permit users to specify callbacks to invoke when specific errors occur, or possibly a generic callback invoked when any ignored error occurs.

A reasonable default behavior might be to automatically set up these callbacks to simply log the error. That would accomplish the goal of making robust server setup easier, while also ensuring that there's still an easy way to discover and investigate possible bugs--or, in a production environment, actual attempted DoS attacks.

@avkonst

This comment has been minimized.

Copy link

@avkonst avkonst commented Mar 18, 2018

Completely agree with the above comment.

What is the behavior, when timeout is 0 ms?

I consider it totally appropriate to reject new connections and free related resources immediately if they can not be accepted due to lack of handlers allowed. It is unlikely that there will be anyone allowed in 10ms (or whatever small interval), but context for timeouts and inbound connections will be held in RAM.

Upper layer should be notified about such rejections.

@seanmonstar seanmonstar added the B-rfc label Mar 19, 2018
@seanmonstar

This comment has been minimized.

Copy link
Member

@seanmonstar seanmonstar commented Apr 16, 2018

#1488 was just merged, and included some changes that touch this. Here's how it works now:

  • sleep_on_errors is set to be default on
  • When enabled, it will log at the error! level about the failures, so users can still see bad things are happening.
  • The timeout duration itself isn't configurable, it's just 1 second.
  • It can be turned off, and a completely custom strategy can be created by a user just by providing their own "incoming" stream of accepted connections.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.