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

Gracefully handle panics in all request handlers #10715

Open
3 of 5 tasks
dnephin opened this issue Jul 28, 2021 · 2 comments
Open
3 of 5 tasks

Gracefully handle panics in all request handlers #10715

dnephin opened this issue Jul 28, 2021 · 2 comments

Comments

@dnephin
Copy link
Contributor

dnephin commented Jul 28, 2021

http.Server handles panics in request handler goroutines using recover, but other servers used by Consul do not. By handling panics in request handlers we will prevent any panics in request handlers from shutting down the entire process.

@gmichelo
Copy link
Contributor

gmichelo commented Aug 7, 2021

Concerning grpc.Server, what do we want to do with the panic? Just turn it into a codes.Internal error? http.Server also prints the stack trace to stderr. If stack trace is not needed, something like the following should suffice:

gmichelo@2b14a9b

This is a prototype, if this approach is confirmed, I plan to add comments, tests and perhaps try to find a way to make the middleware registration common in a shared function.

@dnephin
Copy link
Contributor Author

dnephin commented Aug 11, 2021

Thank you for working on this!

Returning codes.Internal and printing the stack trace using logger.Warn (or logger.Error) sounds like a good way of handling a panic. That diff also looks like a good start!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants