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

Issue #2690 - clarify websocket message ordering in documentation #7097

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

lachlan-roberts
Copy link
Contributor

closes #2690

@@ -41,3 +41,4 @@ If your application needs specific features that are not provided by the standar
include::server-websocket-standard.adoc[]
include::server-websocket-jetty.adoc[]
include::server-websocket-filter.adoc[]
include::../../websocket.adoc[]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbordet I thought websocket.adoc was the common section which was supposed to be in both client and server websocket documentation. I couldn't see it in the Server side documentation I have added this here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's the right place, as it speaks about Jetty-specific annotations and classes, so perhaps it should be an include inside server-websocket-jetty.adoc.

I could not write a generic WebSocket architecture, since you have to refer to annotation and classes that are specific.

Also, including it in the server will cause duplicate links, so perhaps for the server documentation I would just add a paragraph that says something like "for the Jetty WebSocket architecture read xref:..." and point to the client docs (rather than including it again).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have reverted this change, looks like there is already some links to it from the Server documentation. Just seems strange that the common websocket documentation is contained within the client section.

@@ -46,6 +46,8 @@ This event is emitted when the WebSocket communication encounters a fatal error,
Applications interested in the error event receive a `Throwable` that represent the error.
* The _message_ event.
The message event is emitted when a WebSocket message is received.
Only a single thread will be delivering a message event at any one time, and the next message event will not be triggered until the previous one has exited its onMessage method.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Only a single thread will be delivering a message event at any one time, and the next message event will not be triggered until the previous one has exited its onMessage method.
Only one thread at a time will be delivering a message event to the `@OnMessage` method; the next message event will not be delivered until the previous call to the `@OnMessage` method has exited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to use that specific @OnMessage annotation because this that annotation is specific to javax.websocket. This is talking about a more general message event which could be a method from a listener class or the @OnWebSocketMessage annotation from Jetty API.

@@ -41,3 +41,4 @@ If your application needs specific features that are not provided by the standar
include::server-websocket-standard.adoc[]
include::server-websocket-jetty.adoc[]
include::server-websocket-filter.adoc[]
include::../../websocket.adoc[]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's the right place, as it speaks about Jetty-specific annotations and classes, so perhaps it should be an include inside server-websocket-jetty.adoc.

I could not write a generic WebSocket architecture, since you have to refer to annotation and classes that are specific.

Also, including it in the server will cause duplicate links, so perhaps for the server documentation I would just add a paragraph that says something like "for the Jetty WebSocket architecture read xref:..." and point to the client docs (rather than including it again).

@lachlan-roberts lachlan-roberts merged commit 15c1e08 into jetty-10.0.x Nov 16, 2021
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-2690-wsMessageOrderingDocs branch November 16, 2021 05:26
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.

Clarify ordering, concurrency behavior of WebSocketListener and normal HTTP responses
2 participants