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.

Sign up for GitHub

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

Jump to bottom

Re-panic after recovering in Recovery middleware #37

Open
ejholmes opened this issue Apr 14, 2016 · 0 comments
Open

Re-panic after recovering in Recovery middleware #37

ejholmes opened this issue Apr 14, 2016 · 0 comments
Labels
enhancement

Comments

@ejholmes
Copy link
Contributor

I think recovering from panics in https://github.com/remind101/pkg/blob/master/httpx/middleware/recovery.go#L41-L51 should be an anti pattern, for a couple reasons:

  1. In general, it's bad practice to panic in the first place. Normal errors should be returned as an error type.
  2. It makes debugging panics in dev really annoying, because you don't get the full stack trace.

I think the recovery middleware still has value, it should recover from the panic, report it to the reporter, then re-panic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ejholmes