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 statusCodeTracker to get inner http.ResponseWriter #34

Closed
mpuncel opened this issue Dec 21, 2018 · 2 comments
Closed

Comments

@mpuncel
Copy link

mpuncel commented Dec 21, 2018

Existing HTTP handler code that needs to get at the underlying http.Flusher (or http.Hijacker etc) interfaces from the http.ResponseWriter breaks when the Middleware is installed.

One complex approach to address this would be to do something similar to https://github.com/DataDog/dd-trace-go/blob/v1/contrib/internal/httputil/trace_gen.go#L18.

A more simple approach would be to add something like

+func (w *statusCodeTracker) InnerResponseWriter() http.ResponseWriter {
+       return w.ResponseWriter
+}
+
+type ResponseWriterWrapper interface {
+       InnerResponseWriter() http.ResponseWriter
+}

The existing code can then be modified to first try to cast to a nethttp.ResponseWriterWrapper, unwrap it if it works, and then get the underlying http.Flusher etc. from that. I'll submit a patch using this approach and we can discuss from there.

mpuncel added a commit to mpuncel/go-stdlib that referenced this issue 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 opentracing-contrib#34
mpuncel added a commit to mpuncel/go-stdlib that referenced this issue 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 opentracing-contrib#34
mpuncel added a commit to mpuncel/go-stdlib that referenced this issue 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 opentracing-contrib#34
@yurishkuro
Copy link
Collaborator

I believe this is addressed by #26, closing. Feel free to reopen if not.

@mpuncel
Copy link
Author

mpuncel commented Dec 22, 2018

that approach is better than my initial suggestion and solves my use case, thanks for merging that!

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