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

report.JSON breaks websockets #29

Open
ohookins opened this issue Jan 10, 2017 · 4 comments
Open

report.JSON breaks websockets #29

ohookins opened this issue Jan 10, 2017 · 4 comments

Comments

@ohookins
Copy link
Contributor

If you wrap your websocket Handler in report.JSON you end up with something like this:

2017/01/10 14:15:43 websocket: response does not implement http.Hijacker
@telendt
Copy link

telendt commented Jan 11, 2017

@ohookins: That's quite common problem, not only present in handy but in most libs wrapping various "upgradable" interfaces.
Here's a good read about upgradeable interfaces, the problems they solve and the ones they bring:
https://avtok.com/2014/11/05/interface-upgrades.html

This PR solves it for gzipWriter. But there are more interfaces which ResponseWriter can be upgraded to...

@ohookins
Copy link
Contributor Author

@telendt I guess that's just a simpler version of this:
https://github.com/prometheus/client_golang/blob/master/prometheus/http.go#L296-L306

@ohookins
Copy link
Contributor Author

@streadway are you ok with me making a PR for report along the lines of #28 ? Or perhaps @telendt is kind enough to add report to the same PR...

@streadway
Copy link
Owner

Go for it. There are a few more interfaces to implement as well for http/2 support.

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

3 participants