<< Back to previous view

[WEBSOCKET_SPEC-174] Add more detail to javax.websocket.Session JavaDoc about which methods do not throw IllegalStateException Created: 19/Mar/13  Updated: 07/Dec/13  Resolved: 21/Mar/13

Status: Resolved
Project: websocket-spec
Component/s: None
Affects Version/s: 1.0
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Nick Williams Assignee: dannycoward
Resolution: Fixed Votes: 0
Remaining Estimate: 15 minutes
Time Spent: Not Specified
Original Estimate: 15 minutes

Tags: final
Participants: dannycoward and Nick Williams

 Description   

Current JavaDoc says this:

Once the session is closed, it is no longer valid for use by applications. Calling any of its methods once the session has been closed will result in an IllegalStateException being thrown. Developers should retrieve any information from the session during the Endpoint.onClose(javax.websocket.Session, javax.websocket.CloseReason) method.

I believe this needs more clarification. I submit the following:

  • close() and close(CloseReason), in accordance with the java.io.Closeable contract, must NOT throw an exception if the Session is already closed, but instead calling these after Session close should be a no-op. Since this is specified already in Closeable, it may or may not be necessary to specify this in Session, but I suggest it doesn't hurt, and makes it 100% unambiguous.
  • isOpen() should NOT throw an exception, since it is used to indicate whether the Session is open (and, thus, whether a subsequent call to some other method is unlikely to throw an IllegalStateException).
  • getId() should NOT throw an exception, since the String it returns cannot be acted upon, and the ID could be useful debugging/tracking/logging/troubleshooting information post-close.


 Comments   
Comment by Nick Williams [ 19/Mar/13 04:09 PM ]

Now that I think about it, even more clarification is needed. Consider this: Whether I call Session#close(...) or the remote endpoint closes the Session, it is not valid to send messages on the remote endpoint in Endpoint#onClose(...). But the JavaDoc implicitly suggests that Endpoint#onClose(...) can call any Session method without throwing an IllegalStateException. I do not believe this is correct. For the most part, Session methods shouldn't throw IllegalStateException until AFTER Endpoint#onClose(...) returns. However, I believe the following methods should throw IllegalStateExceptions if called from Endpoint#onClose(...):

  • addMessageHandler(MessageHandler)
  • removeMessageHandler(MessageHandler)
  • Any set* methods.
  • getAsyncRemote()
  • getBasicRemote()

This can simply be handled by (internally) assuming the session has three states: OPEN, CLOSING and CLOSED. It's OPEN until close(...) is called locally or remotely, then it's CLOSING until Endpoint#onClose(...) returns, then it's CLOSED. The methods mentioned in this comment throw an IllegalStateException if the session state != OPEN, the rest of the methods throw an IllegalStateException if the session state == CLOSED, except for the four exceptions noted above, which never throw IllegalStateException.

Thoughts?

Comment by dannycoward [ 21/Mar/13 11:56 PM ]

I think you're right. I've clarified that during on close developers can access the session information (id, user data etc), but should not try to modify any of the data, nor try to send messages.

I think that covers the cases that could cause issues.

I think CLOSING is a good way to model it internally, although there are even two sorts of closing: one is closing initiated locally, and one is closing after receipt of a close frame from the client.

Comment by Nick Williams [ 15/Apr/13 04:12 PM ]

I think we need slightly more clarification still. See this bug:

https://issues.apache.org/bugzilla/show_bug.cgi?id=54746

I must be able to call certain methods to obtain information from a session (getId, getRequestParameterMap, getRequestURI, getUserPrincipal, getUserProperties, getContainer, getPathParameters, getQueryString) even after a session is closed. This is especially important if onError is called after a session is closed (as is the case with this bug). The data provided by these methods is crucial to log an error and help track down its cause.

I propose that these eight methods be made to NEVER throw an IllegalStateException. No benefit is gained from throwing an IllegalStateException in these methods after the Session is closed, and no danger is created by not throwing an IllegalStateException. These methods are by their very nature idempotent.

Comment by Nick Williams [ 07/Dec/13 04:08 PM ]

I would love to get this further clarified for Java EE 8, as per the comment above.

Generated at Sat Apr 19 06:55:31 UTC 2014 using JIRA 4.0.2#472.