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

Websocket proxy #146

Merged
merged 1 commit into from
Oct 23, 2016
Merged

Websocket proxy #146

merged 1 commit into from
Oct 23, 2016

Conversation

johartl
Copy link
Collaborator

@johartl johartl commented Oct 23, 2016

Sorry, yet another websocket proxy fix :/ The old proxy middleware we were using did not properly support websockets.
I tested this change with PokeMap-2 and the websocket connection was established successfully. For PokeMap-1 the websocket connection is still not working due to PokemonGoers/PokeMap-1#42.

Copy link
Member

@WoH WoH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G2G

@johartl johartl merged commit c072b3f into develop Oct 23, 2016
@johartl johartl deleted the websocket-proxy-fix branch October 23, 2016 17:17
@sacdallago
Copy link
Member

Sorry :D yesterday I stopped before publishing the websocket port via nginx as I was having issues on other things and wanted to go step by step.. It's very strange that it works.. VERY strange.. My assumption is that it doesn't make use of the env var I pass it on when building, but now with longer logging I might have time to look into it

@johartl
Copy link
Collaborator Author

johartl commented Oct 24, 2016

My assumption is that it doesn't make use of the env var I pass it on when building

That makes sense as the websocket env variable defaults to the old docker container (which is working just fine) .
The bug I fixed with this PR, however, had nothing to do with the setup of the production system. I exclusively tested it with the old docker container so this is something we would've needed to fix anyway ;)

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.

3 participants