Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      Session class documentation does not mention how it should work after Session.close(...) is called. What should happen when Session.getRemote() is called?

      Similar case can happen in annotated endpoint scenario - consider following:

              @WebSocketMessage
              public String message(String message, Session session) {
                  try {
                      session.close();
                      return ...; // ???
                  } catch (IOException e) {
                  }
                  return "message";
              }

      line "return ...;" is important here.

      What should be returned?

        Activity

        Hide
        dannycoward added a comment -

        still discussing in expert group.

        Show
        dannycoward added a comment - still discussing in expert group.
        Hide
        dannycoward added a comment -

        so I think we have decided that invalid sessions throw IllegalArgumentExceptions if their methods are called, so sesssion.getRemote() will throw that if called after the session is closed.

        The other part of this is what happens in the example code.

        The method has to return (assuming ... is a string or something that itself does not throw an error. if it does throw an error, that's handled by the error handling mechanism).

        So what does the container do with the return value ?

        IMO, it would be bad form for the container to swallow it. So I think some kind of error fed back into the error handler is most appropriate here.

        Show
        dannycoward added a comment - so I think we have decided that invalid sessions throw IllegalArgumentExceptions if their methods are called, so sesssion.getRemote() will throw that if called after the session is closed. The other part of this is what happens in the example code. The method has to return (assuming ... is a string or something that itself does not throw an error. if it does throw an error, that's handled by the error handling mechanism). So what does the container do with the return value ? IMO, it would be bad form for the container to swallow it. So I think some kind of error fed back into the error handler is most appropriate here.
        Hide
        Pavel Bucek added a comment -

        "..." has to be null or String (wouldn't be compilable otherwise).

        Current state is that:

        • if null is returned, everything is fine
        • if something else, onError is called (because somebody tried to write to closed connection).

        What I had in mind when I was filing this issue was "This state is not documented anywhere; I want to call session.close() and logically, I should not return anything, but I have to (won't compile without it).".

        This can be fixed by adding appropriate statement(s) to @WebSocketMessage and MessageHandler classes javadoc.

        Show
        Pavel Bucek added a comment - "..." has to be null or String (wouldn't be compilable otherwise). Current state is that: if null is returned, everything is fine if something else, onError is called (because somebody tried to write to closed connection). What I had in mind when I was filing this issue was "This state is not documented anywhere; I want to call session.close() and logically, I should not return anything, but I have to (won't compile without it).". This can be fixed by adding appropriate statement(s) to @WebSocketMessage and MessageHandler classes javadoc.
        Hide
        dannycoward added a comment -

        OK I see.

        Then the current state matches what was discussed in the expert group - good.

        Yes, its unavoidable that the method has to complete with something returned. Java is so static sometimes !

        I can certainly clarify the behavior in @WebSocketMessage.

        What's the issue in the MessageHandler ? There's no return out of the onMessage() calls.

        Show
        dannycoward added a comment - OK I see. Then the current state matches what was discussed in the expert group - good. Yes, its unavoidable that the method has to complete with something returned. Java is so static sometimes ! I can certainly clarify the behavior in @WebSocketMessage. What's the issue in the MessageHandler ? There's no return out of the onMessage() calls.
        Hide
        Pavel Bucek added a comment -

        Sorry, please disregard that; MessageHandler should be RemoteEndpoint in last comment but when I'm reading its documentation, it looks like there are no conflicts with what was written here. Session methods might have some indication about what will be returned when it is in "open" state and what will happen when it is in "close" state, but IllegalStateException can always be thrown, so that's really minor thing.

        Show
        Pavel Bucek added a comment - Sorry, please disregard that; MessageHandler should be RemoteEndpoint in last comment but when I'm reading its documentation, it looks like there are no conflicts with what was written here. Session methods might have some indication about what will be returned when it is in "open" state and what will happen when it is in "close" state, but IllegalStateException can always be thrown, so that's really minor thing.
        Hide
        dannycoward added a comment -

        OK no worries. WebSocketMessage is now updated, and I added a class level statement in RemoteEndpoint that should deal with the situation of calling a RemoteEndpoint method after the session is closed.

        Show
        dannycoward added a comment - OK no worries. WebSocketMessage is now updated, and I added a class level statement in RemoteEndpoint that should deal with the situation of calling a RemoteEndpoint method after the session is closed.

          People

          • Assignee:
            dannycoward
            Reporter:
            Pavel Bucek
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved: