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

Use of Session (add|remove|get)MessageHandler from annotated endpoint #192

Closed
glassfishrobot opened this issue Apr 18, 2013 · 6 comments
Closed
Labels
API (Both) Impacts the client and server API bug Something isn't working Jakarta EE 10

Comments

@glassfishrobot
Copy link

As reported on jsr356-experts:
http://java.net/projects/websocket-spec/lists/jsr356-experts/archive/2013-04/message/5

(snip)
If you have an annotated endpoint via @ClientEndpoint or @serverendpoint, can you use the various Session MessageHandler calls?

Session.addMessageHandler()

This would seem to makes sense, but the rules for replacing a "native websocket message type" should still apply, right?
So that if we have a @OnMessage annotated method with a TEXT type (such as String) then Session.addMessageHandler(MessageHandler.Whole) shouldn't work.

Session.removeMessageHandler()

Should we be able to remove a @OnMessage annotated method?
Perhaps by exposing these annotated methods a implementation specific MessageHandlers?

Session.getMessageHandlers()

Should this also list the annotated @OnMessage methods as MessageHandlers?

It could be argued that it is confusing to have Session.addMessageHandler() fail with a reason that the the native websocket message type is already in use, but not have the means to remove the active one via removeMessageHandler(), or have the means to list them via getMessageHandlers().
(/snip)

This is another area needing clarification in the spec.

A proposal is to have to have the following methods not be available for Annotated WebSocket Endpoints.

Session.addMessageHandler()
Session.getMessageHandlers()
Session.removeMessageHandlers()

Throwing IllegalStateException when attempting to use them.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Reported by joakimerdfelt

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
gregwilkins said:
Is there really a need to allow ongoing add/remove of message handlers?

I would think that it is sufficient to have only add and to allow it only during a call to onOpen. After that, the handlers should be fixed and thus it does not matter if they are annotated or otherwise.

In cometd, we had a lot of problems with this kind of race - when we would notify an application of a new channel subscription which would often result in new message handlers being registered. If this is not made atomic, then it is always possible for messages to arrive before the message handler is registered. The solution in cometd was to have the mutable methods (eg addMessageHandler) only on a version of the interface that is only passed to the equivalent of onOpen.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
This issue was imported from java.net JIRA WEBSOCKET_SPEC-192

@glassfishrobot
Copy link
Author

@markt-asf
Copy link
Contributor

This is going to have to wait until Jakarta EE 10 for clarification. Given the time the existing API has been in used, I'm leaning towards an atomic replacement approach. Messages being handled in the replaced handler would continue normally and the next message will be processed by the replacement. One query - what about partial messages? Continue with the old handler or switch to the new? I'm thinking continue with the old but we have plenty of time to discuss this.

@markt-asf markt-asf added API (Both) Impacts the client and server API bug Something isn't working Jakarta EE 10 and removed Priority: Major Type: Bug labels May 15, 2020
markt-asf added a commit to markt-asf/websocket-api that referenced this issue Sep 30, 2021
…ler add/remove

To quote gregw "Once you start a websocket message, you continue to use
the same MessageHandler for the entire message."
markt-asf added a commit to markt-asf/websocket-api that referenced this issue Sep 30, 2021
…ler add/remove

To quote gregw "Once you start a websocket message, you continue to use
the same MessageHandler for the entire message."
markt-asf added a commit to markt-asf/websocket-api that referenced this issue Oct 1, 2021
…ler add/remove

To quote gregw "Once you start a websocket message, you continue to use
the same MessageHandler for the entire message."
markt-asf added a commit that referenced this issue Oct 1, 2021
To quote gregw "Once you start a websocket message, you continue to use
the same MessageHandler for the entire message."
@markt-asf
Copy link
Contributor

Fixed by #369.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API (Both) Impacts the client and server API bug Something isn't working Jakarta EE 10
Projects
None yet
Development

No branches or pull requests

2 participants