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

New supportsCustomHeaders property in WebSocketClient interface #474

Closed
joffrey-bion opened this issue Mar 17, 2024 · 1 comment
Closed
Labels
breaking A breaking change

Comments

@joffrey-bion
Copy link
Owner

joffrey-bion commented Mar 17, 2024

Some WebSocketClient implementations do not support sending custom headers in the handshake, and this cannot be implemented in Krossbow. Here are some cases where it's just not possible right now:

Calling connect() with a non-empty headers map on these clients currently causes an IllegalArgumentException. It would be nice to allow consumers to check up front whether the client supports custom headers.

Possible implementations:

  • a new property supportsCustomHeaders: Boolean on WebSocketClient
  • 2 different WebSocketClient types, one with the connect() method that takes a headers param and one that doesn't.

It would also simplify tests.

@joffrey-bion joffrey-bion added enhancement New feature or request breaking A breaking change labels Mar 17, 2024
@joffrey-bion
Copy link
Owner Author

Use different types in generally more appealing, because the API is not "lying" by advertising a headers parameter that is not respected and fails the call.

However, it's not always technically possible to implement different interfaces when the header support of the implementation depends on factors that Krossbow doesn't control. For instance, when wrapping Ktor's web socket, the support depends on the engine, which is not statically known.

We'll have to go the other way: using a property.

@joffrey-bion joffrey-bion removed the enhancement New feature or request label Mar 19, 2024
@joffrey-bion joffrey-bion changed the title Add a way to know whether a WebSocketClient supports custom headers New supportsCustomHeaders property in WebSocketClient interface Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking change
Projects
None yet
Development

No branches or pull requests

1 participant