Is panic a problem in fasthttp

[dgrr]

I am seeing a lot of panics at fasthttp and a lot of them are common like: https://github.com/erikdubbelboer/fasthttp/blob/master/server.go#L1700

It would be a good idea to replace it by error type in return statements?

[illotum]

If you check https://golang.org/src/net/net.go#L239, you'll find errors here are either related to a missing connection (it's maybe okay, just shutdown HTTP scaffolding around it) or syscall error (it's not okay, something is off with the OS).

Overall, both indicate serious problem and I would rather see a panic here. It's obviously on a case-by-case basis and I am a fan of fail-fast designs. Ultimately this is up to a library author, valyala in our case.

[erikdubbelboer]

I guess the reason @valyala used a panic here is because it means something is seriously wrong. In all my years of fasthttp usage with thousands of requests per second each and every day I have never seen this panic happen. What exactly are you doing to cause this panic?

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.

[dgrr]

And what about this panic https://github.com/erikdubbelboer/fasthttp/blob/master/http.go#L1647 It will be simple to return the error and log it in server logger.

[erikdubbelboer]

I don't even think that code is reachable unless bufio.Reader or the reader it is buffering from are broken. As far as I know a normal io.Reader.Read() will never return 0 bytes without also returning an EOF error.

[thehowl]

Well, it is discouraged behaviour, but technically allowed in the Go doc:

Implementations of Read are discouraged from returning a zero byte count with a nil error, except when len(p) == 0. Callers should treat a return of 0 and nil as indicating that nothing happened; in particular it does not indicate EOF.

[erikdubbelboer]

I actual see a bigger bug in this code. It only checks err if nn <= 0. The io.Reader doc says When Read encounters an error or end-of-file condition after successfully reading n > 0 bytes, it returns the number of bytes read.. So it's very likely that errors don't have nn <= 0.

[thehowl]

That should not be a bug, but rather a missed optimisation:

It may return the (non-nil) error from the same call or return the error (and n == 0) from a subsequent call.

In other words, if we miss the error on the first run because n > 0, we will still get it in the run after that.