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

Add ability to unwrap ResponseWriter set by Middleware #35

Conversation

mpuncel
Copy link

@mpuncel mpuncel commented Dec 21, 2018

When wrapping an HTTP server with Middleware(), HTTP handlers lose the
ability to cast their ResponseWriter argument to any of the optional
interfaces like http.Flusher or http.Hijacker. This commit adds an
optional interface ResponseWriterWrapper which has a method for
unwrapping the ResponseWriter installed by Middleware.

Existing handlers can then test for implementation of this interface and
do an unwrap if the conversion succeeds, and then use whatever
underlying optional http interfaces were previously accessible.

Addresses #34

@mpuncel mpuncel force-pushed the mpuncel/unwrap-response-writer branch from 5fe6a3d to 89669d3 Compare December 21, 2018 17:54
When wrapping an HTTP server with Middleware(), HTTP handlers lose the
ability to cast their ResponseWriter argument to any of the optional
interfaces like http.Flusher or http.Hijacker. This commit adds an
optional interface ResponseWriterWrapper which has a method for
unwrapping the ResponseWriter installed by Middleware.

Existing handlers can then test for implementation of this interface and
do an unwrap if the conversion succeeds, and then use whatever
underlying optional http interfaces were previously accessible.

Addresses opentracing-contrib#34
@mpuncel mpuncel force-pushed the mpuncel/unwrap-response-writer branch from 89669d3 to ee969ee Compare December 21, 2018 18:07
@mpuncel
Copy link
Author

mpuncel commented Dec 21, 2018

I'm realizing the approach here might not be sufficient for many use cases, for instance if the type assertions happen in third party code.

@yurishkuro
Copy link
Collaborator

I am not sure how this would help. The various standard interfaces that a writer can implement are often checked by 3rd party libraries, which would have no knowledge of this extra unwrap method.

@mpuncel
Copy link
Author

mpuncel commented Dec 22, 2018

yep, will close. I think an approach like https://github.com/felixge/httpsnoop would be better

@mpuncel mpuncel closed this Dec 22, 2018
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.

2 participants