Skip to content

Conversation

ThisIsMissEm
Copy link

I'm using this gem for some websocket tests in the Mastodon codebase, where we have support for authentication by abusing websocket subprotocols. Currently it's only possible to set the protocols when you initialize the WebSocket client, but not afterwards before starting the client, this means you have to know the protocols value immediately, not lazily.

This change to add support for setting the protocols after initialization but before start ended up being a bit gnarly, due to the way the headers worked.

I had to split the set_header headers from the @headers because the tests all failed due to expecting headers in a specific line order, which became messed up otherwise

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure why this file needs

headers = [start, @custom_headers.merge(@headers).to_s, '']

when Client needs:

headers = [start, @headers.merge(@custom_headers).to_s, '']

@jcoglan
Copy link
Collaborator

jcoglan commented Sep 6, 2025

Hi @ThisIsMissEm 👋 Could you show me an example of how you're using this new method? I'm not sure I understand why you have information that needs to be supplied before start is called to do the handshake, but it can't be given to the constructor.

@ThisIsMissEm
Copy link
Author

@jcoglan I ended up restructuring my code and starting the driver instantiation later, but you can see what I was doing in: mastodon/mastodon#36025

@jcoglan
Copy link
Collaborator

jcoglan commented Sep 7, 2025

So, is this change still needed? In the linked PR, you pass the protocols to the Driver.client constructor and I can't see anything unusual being done with the @driver object.

I could imagine wanting this on the server end, where you set the protocols based on some header or other input from the client, but on the client side, the driver is initiating the handshake so there's no other input from the other end to influence the choice of protocols.

@ThisIsMissEm
Copy link
Author

Yeah, I originally had something closer to your example code and was getting caught with needing to initialise the driver with auth via subprotocol, now I create the driver on connect, allowing me to defer things

So this could be closed

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