Re: Currently ignored WebSocket tests
On 08/01/2015 16:44, Rémy Maucherat wrote:
2015-01-08 16:56 GMT+01:00 Mark Thomas ma...@apache.org:
There are a couple of WebSocket tests that are currently commented out
in org.apache.tomcat.websocket.pojo.TestEncodingDecoding
I've been looking into why the currently fail.
Failing test 1: testMessagesEndPoints()
First of all, this test fails with:
java.lang.IllegalStateException: The remote endpoint was in state
[TEXT_FULL_WRITING] which is an invalid state for called method
This is caused by a bug in the server-side endpoint which has this:
@OnMessage
public String onMessage(String message, Session session) {
received.add(message);
session.getAsyncRemote().sendText(MESSAGE_ONE);
return message;
}
The problem is that because the method is annotated with @OnMessage and
has a return value, that return value will be sent as a message to the
client. However, just before the method ends an asynchronous message is
send with:
session.getAsyncRemote().sendText(MESSAGE_ONE);
The first (asynchronous) message has not completed when the second (from
the return value) message is attempted to be sent.
The Javadoc for RemoteEndpoint.Basic states:
If the websocket connection underlying this RemoteEndpoint is busy
sending a message when a call is made to send another one, for example
if two threads attempt to call a send method concurrently, or if a
developer attempts to send a new message while in the middle of sending
an existing one, the send method called while the connection is already
busy may throw an IllegalStateException.
It is arguable (from the Javadoc) that this limitation only applies to
synchronous messages but I do not believe that that was the intention of
the WebSocket EG. It was certainly only ever discussed in the context
sync+async messages.
If the above issue is fixed (e.g. by changing the return type to void)
the test then fails at the various calls to testEvent. This is because
the test does not use the encoders those checks are looking for.
I think I already mentioned that. In the Tomcat implementation, this means
mixing async with blocking (which obviously fails), but sending the
returned value is under the control of the container so it's an
implementation detail. Thus I see where the rationale comes from.
You could discuss it in the EG.
Failing test 2: testBatchedEndPoints()
This fails because batching is enabled which means that the messages are
batched (i.e. buffered) until the buffer is full and the two messages
written do no fill the buffer. If this were addressed then again the
test will still fail because of the later calls to testEvent. Again the
test does not use the encoders these checks are looking for.
Yes, it seems implied the value returned should not be concerned with
batching (it would probably never be sent) and thus the first batched
message would also be sent at that time. This sounds easier to fix than the
first one, but I didn't really try because the behavior should be validated
first.
I'll ask some questions in the EG.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Currently ignored WebSocket tests
2015-01-08 16:56 GMT+01:00 Mark Thomas ma...@apache.org:
There are a couple of WebSocket tests that are currently commented out
in org.apache.tomcat.websocket.pojo.TestEncodingDecoding
I've been looking into why the currently fail.
Failing test 1: testMessagesEndPoints()
First of all, this test fails with:
java.lang.IllegalStateException: The remote endpoint was in state
[TEXT_FULL_WRITING] which is an invalid state for called method
This is caused by a bug in the server-side endpoint which has this:
@OnMessage
public String onMessage(String message, Session session) {
received.add(message);
session.getAsyncRemote().sendText(MESSAGE_ONE);
return message;
}
The problem is that because the method is annotated with @OnMessage and
has a return value, that return value will be sent as a message to the
client. However, just before the method ends an asynchronous message is
send with:
session.getAsyncRemote().sendText(MESSAGE_ONE);
The first (asynchronous) message has not completed when the second (from
the return value) message is attempted to be sent.
The Javadoc for RemoteEndpoint.Basic states:
If the websocket connection underlying this RemoteEndpoint is busy
sending a message when a call is made to send another one, for example
if two threads attempt to call a send method concurrently, or if a
developer attempts to send a new message while in the middle of sending
an existing one, the send method called while the connection is already
busy may throw an IllegalStateException.
It is arguable (from the Javadoc) that this limitation only applies to
synchronous messages but I do not believe that that was the intention of
the WebSocket EG. It was certainly only ever discussed in the context
sync+async messages.
If the above issue is fixed (e.g. by changing the return type to void)
the test then fails at the various calls to testEvent. This is because
the test does not use the encoders those checks are looking for.
I think I already mentioned that. In the Tomcat implementation, this means
mixing async with blocking (which obviously fails), but sending the
returned value is under the control of the container so it's an
implementation detail. Thus I see where the rationale comes from.
You could discuss it in the EG.
Failing test 2: testBatchedEndPoints()
This fails because batching is enabled which means that the messages are
batched (i.e. buffered) until the buffer is full and the two messages
written do no fill the buffer. If this were addressed then again the
test will still fail because of the later calls to testEvent. Again the
test does not use the encoders these checks are looking for.
Yes, it seems implied the value returned should not be concerned with
batching (it would probably never be sent) and thus the first batched
message would also be sent at that time. This sounds easier to fix than the
first one, but I didn't really try because the behavior should be validated
first.
Proposed fixes:
I will removed the testEvent checks since they do not apply to these tests.
I believe these tests were aiming to reproduce issues Remy had seen in
other systems. Since I don't have access to the original issues I'll
leave the remainder of the tests as is to give Remy a chance to review
these comments and possibly modify the tests.
Obviously, the end of the tests (that is now removed) was a cut paste :)
Rémy
Currently ignored WebSocket tests
There are a couple of WebSocket tests that are currently commented out
in org.apache.tomcat.websocket.pojo.TestEncodingDecoding
I've been looking into why the currently fail.
Failing test 1: testMessagesEndPoints()
First of all, this test fails with:
java.lang.IllegalStateException: The remote endpoint was in state
[TEXT_FULL_WRITING] which is an invalid state for called method
This is caused by a bug in the server-side endpoint which has this:
@OnMessage
public String onMessage(String message, Session session) {
received.add(message);
session.getAsyncRemote().sendText(MESSAGE_ONE);
return message;
}
The problem is that because the method is annotated with @OnMessage and
has a return value, that return value will be sent as a message to the
client. However, just before the method ends an asynchronous message is
send with:
session.getAsyncRemote().sendText(MESSAGE_ONE);
The first (asynchronous) message has not completed when the second (from
the return value) message is attempted to be sent.
The Javadoc for RemoteEndpoint.Basic states:
If the websocket connection underlying this RemoteEndpoint is busy
sending a message when a call is made to send another one, for example
if two threads attempt to call a send method concurrently, or if a
developer attempts to send a new message while in the middle of sending
an existing one, the send method called while the connection is already
busy may throw an IllegalStateException.
It is arguable (from the Javadoc) that this limitation only applies to
synchronous messages but I do not believe that that was the intention of
the WebSocket EG. It was certainly only ever discussed in the context
sync+async messages.
If the above issue is fixed (e.g. by changing the return type to void)
the test then fails at the various calls to testEvent. This is because
the test does not use the encoders those checks are looking for.
Failing test 2: testBatchedEndPoints()
This fails because batching is enabled which means that the messages are
batched (i.e. buffered) until the buffer is full and the two messages
written do no fill the buffer. If this were addressed then again the
test will still fail because of the later calls to testEvent. Again the
test does not use the encoders these checks are looking for.
Proposed fixes:
I will removed the testEvent checks since they do not apply to these tests.
I believe these tests were aiming to reproduce issues Remy had seen in
other systems. Since I don't have access to the original issues I'll
leave the remainder of the tests as is to give Remy a chance to review
these comments and possibly modify the tests.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
