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

Handlers that spin off goroutines should recover, log, and send error #91

Open
ScottMansfield opened this issue Aug 16, 2016 · 1 comment

Comments

@ScottMansfield
Copy link
Contributor

There's a few places in the code base that do not have a proper recover function to handle panics. Any place with a realHandle* function is probably spinning off a goroutine that could panic If that goroutine panics, it will kill the entire process.

Any place where a recover is placed, there are channels for returning errors and data. There is a decision to be made here: Do we panic and allow everything to continue as usual or do we return an error?

The goroutine needs to return an error on the errchan. If there's a panic, the connection to the backing store could be in a broken state. If we return an error the connection will eventually get closed along with the other related connections (including the external one). This is desirable because it guarantees a clean starting state when a client reconnects.

While doing the recover gymnastics, a metric for caught panics should be incremented and a log line should be generated so that any panic location is tracked. There is already an unrecoverable error metric tracked in the server, so another metric isn't necessary.

@danishprakash
Copy link

@ScottMansfield would like to work on this. You've already given ample information but given that this issue is way too old now, are you still accepting contributions?

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