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: use empty array for ws protocols instead of null #2140

Merged
merged 1 commit into from
May 6, 2022

Conversation

tsmethurst
Copy link
Contributor

Heya! I was running into a bug using Pinafore with GoToSocial, but only under Chrome.

Specifically, was getting the following error:

Error during WebSocket handshake: Sent non-empty 'Sec-WebSocket-Protocol' header but no response was received

I looked at the request headers that were being sent by the websocket connection, and saw 'Sec-Websocket-Protocol: null`.

After a bit of digging through the code, I noticed that if you pass 'null' to the WebSocket client constructor, it will pass 'null' via the headers, whereas if you just pass an empty array by default, this header is skipped.

This seems to resolve the issue :) Tested with Chrome and GtS

@tsmethurst tsmethurst changed the title [bugfix] Use empty array for ws protocols instead of null fix: use empty array for ws protocols instead of null May 6, 2022
@nolanlawson
Copy link
Owner

Thanks for reaching out. Yeah looks like I should either be omitting the parameter or using the empty array: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/WebSocket

If it is omitted, an empty array is used by default, i.e. [].

I'm surprised this hasn't caused problems before, but looks like a good fix. Thanks!

@tsmethurst
Copy link
Contributor Author

tsmethurst commented May 6, 2022 via email

@nolanlawson nolanlawson merged commit b312b3b into nolanlawson:master May 6, 2022
alice-werefox pushed a commit to alice-werefox/sema-werefox-cafe that referenced this pull request Apr 3, 2023
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