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

Fix incorrect version of WebSocketClient instance being used. #14

Merged
merged 1 commit into from Sep 8, 2017

Conversation

fungos
Copy link
Contributor

@fungos fungos commented Aug 19, 2017

API v4 requires the use of NewWebSocketClient4 instead
NewWebSocketClient.
Now that APIv3 is still supported this is not an issue, but this hides
the problem until the APIv3 is officially dropped. Where trying to
connect to APIv4 using this constructor will lead to a panic without
giving clear information about the problem ("websocket: bad handshake").

This can be reproduced by changing "Allow use of API v3 endpoints" to false in a Mattermost instance config.json.

API v4 requires the use of NewWebSocketClient4 instead
NewWebSocketClient.
Now that APIv3 is still supported this is not an issue, but this hides
the problem until the APIv3 is officially dropped. Where trying to
connect to APIv4 using this constructor will lead to a panic without
giving clear information about the problem ("websocket: bad handshake").
@coreyhulen
Copy link
Contributor

Thanks @fungos for the pull request!

Per the CONTRIBUTING.md file displayed when you created this pull request, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?

This is a standard procedure for many open source projects. Your form should be processed within 24 hours and reviewers for your pull request will be able to proceed.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

@fungos
Copy link
Contributor Author

fungos commented Sep 8, 2017

Alright, it is done. Feeling weird about it though, my contribution is literally "4" :)

@coreyhulen coreyhulen merged commit f62f423 into mattermost-community:master Sep 8, 2017
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.

None yet

3 participants