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

allow passing custom header values to recovery #246

Closed
wants to merge 1 commit into from

Conversation

didil
Copy link

@didil didil commented Feb 7, 2019

fixes #241

The solution I found was to have the user pass custom header values to be applied in

(rec *Recovery) ServeHTTP 

before we write the 500 status
hope that's acceptable :)

@jszwedko
Copy link
Contributor

Thanks @didil !

This seems reasonable, but I'm actually beginning to wonder if the provided Recovery middleware should just stick to a basic implementation and to suggest that authors that need additional customization could copy the existing implementation and modify to their specific needs rather than continuing to add configuration to the stock middleware. Especially given the middleware itself is rather small if you take out all of the conditionals / dead code to match exactly what you'd like to output.

This is a bit of a shift to thinking of the provided middlewares as examples rather than something you might run in production.

What do you think?

@didil
Copy link
Author

didil commented Feb 10, 2019

ok I understand ! thanks for the clarification. I'd suggest to close the issue maybe or remove the label "help wanted".

@didil didil closed this Feb 10, 2019
@jszwedko
Copy link
Contributor

👍 thanks @didil . I'll remove the label. I'm still not 100% sure on this stance, but am leaning towards it.

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

Successfully merging this pull request may close these issues.

Can't set response headers when implementing custom Recovery Formatter
2 participants