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

Fix FinalErrorHandler for error phase #4

Merged
merged 1 commit into from
Mar 25, 2016
Merged

Conversation

groyoh
Copy link
Member

@groyoh groyoh commented Mar 25, 2016

Expected behavior

When using a Layer, an unrecovered panic should cause the FinalErrorHandler to be use and return a 500 status code with the error message.

Actual behavior

The FinalErrorHandler was never used if a panic occured. The Run function simply returned without calling serveHTTP (ref. https://github.com/vinci-proxy/layer/blob/master/layer.go#L143) and therefore simply swallows the panic. @h2non correct me if this was the expected behavior.

Additional information

This PR should also increase code coverage.

When no Layer was defined, the FinalErrorHandler was not used when a
panic occured.
@h2non h2non merged commit 72cb7b6 into vinxi:master Mar 25, 2016
@h2non
Copy link
Member

h2non commented Mar 25, 2016

You are right. I think we should include a error specific final handler function to be custimized, istead of using finalHandler. Currently it's bad vecause we are using the same handler for any phase.

@groyoh groyoh deleted the add_tests branch March 25, 2016 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants