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

Pluggable websocket support #704

Open
blasss opened this issue Jan 3, 2024 · 6 comments
Open

Pluggable websocket support #704

blasss opened this issue Jan 3, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@blasss
Copy link

blasss commented Jan 3, 2024

Currently the support for websocket is realized using the Jetty-client which is per se fine and works as expected.
As we use htmlunit in a environment with Selenium and a embedded Jetty for serving our application we struggle with jar versions for our testenvironment. We already updated to Jetty 10 and alternatively use Jetty 11 to prove compatibility with JakartaEE 9.

As far as I can see htmlunit is more or less hardwired to using Jetty 9.x even if there seems to be the idea of plugging different websocket-implementations through WebSocketAdapter. Unfortunately there is no configuration to link a different implementation.

Introducing such a configuration would allow to use different client-implementations and possibly could as well help to scope with issues like #36

@niloc132
Copy link

niloc132 commented Jan 3, 2024

At a quick glance, this looks like a reasonably small piece of work to bite off. It could even be worth proposing using the okhttp websocket implementation as the default for its smaller classpath and lower likelihood of conflicting with other project code - or shading the jetty websocket client into its own package.

What is the usual approach in making something like this configurable in HtmlUnit - at a glance I don't see other cases where implementation details like this can be swapped out at runtime. I'd lean toward a ServiceLoader that picks up whatever it can find, and rely on the user to manage their classpath to ensure they only get the adapter implementation they want?

@rbri
Copy link
Member

rbri commented Feb 10, 2024

as a first start a made the https://github.com/HtmlUnit/htmlunit-websocket-client project.

This only provides a shaded version of the Jetty 9 WebSocket client.

@blasss hope that helps

@rbri
Copy link
Member

rbri commented Feb 10, 2024

@blasss @niloc132

good idea, maybe we can work together here.

  • so far HtmlUnit more or less uses a home brew classpath based approach, so far only to support some differences mainly for android
  • I'd lean toward a ServiceLoader - for me this sounds like a good idea, we can try to move in this direction....

rbri added a commit that referenced this issue Feb 10, 2024
…ded version of the lastes Jetty 9 websocket-client (issue #704)
@rbri
Copy link
Member

rbri commented Feb 10, 2024

@blasss you can try the 3.12.0-SNAPSHOT build

@niloc132
Copy link

I've noticed that Java 17 now provides a websocket API https://docs.oracle.com/en/java/javase/17/docs/api/java.net.http/java/net/http/WebSocket.html - that would be a nice simple option for those who are using java 17. I've done this before with okhttp's websocket too (which works on android and older JDKs), I'll see if I can build these new implementations based on your work here so far.

@blasss
Copy link
Author

blasss commented Feb 19, 2024

Sorry for the long delay, but I didn't manage to test before today.
With the 3.12.0-SNAPSHOT build and the shaded version of Jetty 9 WebSocket client everything works as expected.
With this small adjustment the conflicting version requirements in classpath are avoided.

Configuring the classname of the WebSocketAdapter in WebClientOptions might be a lean alternative to using a fully-fledged ServiceLoader based approach. This would be easy to implement, gives the user full control over the used implementation and avoids confusion if multiple Implementations are available in the classpath.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants