[question/enhancement] Why do we not Recover from panic handlers?

[cipriancraciun]

Currently a handler that panic's will bring down the whole server, thus impacting other outstanding connections and handlers.

Thus I asked myself why doesn't fasthttp protect its connection handler routine with a Recover, thus dropping only that connection? Is it by design?

---

Now (answering and debating with myself) :) I can see both sides of the argument:

• if we don't handle panics, then any small hidden bug would take the whole server down, and if that bug can somehow be easily triggered by the requester, then we end up with a very efficient denial-of-service attack; (even if we use a process supervision, if the server keeps going down, the supervisor will start delaying the restart;)

• however if we do handle panics and just log them, then a major failure in our code would get unnoticed by the supervisor which will prevent a restart that might (temporarily) solve the issue;

---

As such perhaps a "combined" approach can be taken:

• handle the panics and log them;
• count the panics per client IP, and as in the case of requests limit, apply an enforcement; (this will cover an attacker that reliably triggers such a condition;)
• count the panics per whole server, and apply an enforcement; (this will cover a system-wide failure that might be solved by a restart;)

The limits could be either absolute (i.e. this many requests) or percentage (of all served requests). In case of global percentage limit, there should be a small "burst" that would cover any initial temporary failures caused by "initialization" dependencies or failures.

[dgrr]

Probably this will be the better option.

Returning an error instead of a panic would print the error to the logger but keep trying to reuse the connection which isn't going to work.

[erikdubbelboer]

We could add recover() but I'm not sure if it's really up to fasthttp to handle this case for the user. net/http does recover from panics but that's because it has a special mechanism there where a panic aborts the connection. fasthttp doesn't have anything like this (at the moment).

Having users add the recover() to the top of their handler gives the user the choice of how to log the error and how to handle it instead of having fasthttp decide this for the user.

[cipriancraciun]

Fair enough, however this behavior (i.e. crashing the entire server on any panic) is quite different from what one would expect. Therefore might I propose a change in the documentation that clearly states this?

See pull request #475.

[erikdubbelboer]

I merged the pull requests that adds extra documentation.