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

v2: recovery default handler changes #435

Closed
amenzhinsky opened this issue Jul 11, 2021 · 1 comment
Closed

v2: recovery default handler changes #435

amenzhinsky opened this issue Jul 11, 2021 · 1 comment

Comments

@amenzhinsky
Copy link

Hi there.

I'd like to discus the default panic handler in recovery package.

From my experience we always have a top level error handling interceptor that catches all non-grpc errors, sends them to a service like sentry and hides the details from users with status.Error(codes.Internal, "internal error"), so at least it should be fmt.Errorf("panic caught: %v", p).

The second thing is panic stack traces which are really important to track the root cause. The standard http server does it this way.

I do understand that this can be achieved using a custom panic handler but I suppose many people would like this changes and I would stop copy-pasting the following code to every project:

func recoverPanic(p interface{}) error {
	stack := make([]byte, 64<<10)
	stack = stack[:runtime.Stack(stack, false)]
	return &PanicError{Panic: p, Stack: stack}
}

type PanicError struct {
	Panic interface{}
	Stack []byte
}

func (e *PanicError) Error() string {
	return fmt.Sprintf("panic caught: %v\n\n%s", e.Panic, e.Stack)
}
@johanbrandhorst
Copy link
Collaborator

I think that behavior makes sense as the default behavior. Would you like to contribute this change?

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

No branches or pull requests

2 participants