On CONNECT and external broker is not available, StompBrokerRelayMessageHandler should return ERROR frame [SPR-12820]

[spring-projects-issues]

Leonard Siu opened SPR-12820 and commented

According to the Stomp Specification (http://stomp.github.io/stomp-specification-1.1.html#Connecting):

The server can reject any connection attempt. The server SHOULD respond back with an ERROR frame listing why the connection was rejected and then close the connection.

However, at line# 402 of StompBrokerRelayMessageHandler class where the code simply log and error, ignores and return:

SimpMessageType messageType = SimpMessageHeaderAccessor.getMessageType(message.getHeaders());
if (logger.isErrorEnabled() && SimpMessageType.CONNECT.equals(messageType)) {
	logger.error("Broker not active. Ignoring " + message);
}
else if (logger.isDebugEnabled()) {
	logger.debug("Broker not active. Ignoring " + message);
}
return;

---

Affects: 4.1.5

Attachments:

• error message from shutting down activemq.png (18.34 kB)

Referenced from: commits 4886edd

[spring-projects-issues]

Rossen Stoyanchev commented

Good point, we should send an ERROR frame back.

[spring-projects-issues]

Leonard Siu commented

Thanks. Looks Good. Gave the latest 4.2.0.BUILD-SNAPSHOT a try. Got an error frame, as expected, on the client with the following message:

failed to establish TCP connection in session yc96u8_w

Thanks again.

[spring-projects-issues]

Rossen Stoyanchev commented

Thanks for trying this out. Based on the error message however it sounds more like the client actually tried to connect but failed. In other words we still thought the broker was available at the time the CONNECT frame came. So I wonder what the actual sequence in your test was.

I tested by stopping the broker (in my case RabbitMQ) which results almost immediately in a BrokerAvailabilityEvent event set to false and a log message to that extent. Then I try to connect with a client and get an ERROR frame with the message "Broker not available."

[spring-projects-issues]

Leonard Siu commented

I got the above failed to establish TCP connection in session yc96u8_w error using ActiveMQ. I think the reason I got this error is because I never had the ActiveMQ started in the first place.

I tried again, but this time, have ActiveMQ started. Then I stop it to see what happens.

Now I get the error message Connection to broker closed.

I have attached a screenshot capture from Chrome Developer Tools.

!error message from shutting down activemq.png!

However judging from the commit, we should be getting the Broker not available. error message instead?

Either way, this is still better than before, where I would not get any message on the client side at all.

[spring-projects-issues]

Gabriele Del Prete commented

Can this be backported to 4.1.7?

[spring-projects-issues]

Gabriele Del Prete commented

Also... I was reviewing the commit and in the case there's no Handler, the ERROR frame gets transmitted but it does not look like it closes the connection afterwards, if I'm not mistaken (I just recently started studying Spring WebSocket internals, sorry.). Specs say: The server SHOULD respond back with an ERROR frame listing why the connection was rejected and then close the connection.

Please note that if you look into stomp.js source code ( https://github.com/jmesnil/stomp-websocket/blob/master/src/stomp.coffee#L314 ), it does not look like there's special handling for ERROR frames during a CONNECT handshake (there's just an errorCallback invocation), so a client receiving the ERROR frame during a CONNECT won't be automatically close the underlying connection on its side.

I think Spring should close the connection too... ?

[spring-projects-issues]

Gabriele Del Prete commented

Nope, sorry for reopening, I was totally wrong. The connection to the client will be closed by virtue of StompSubProtocolHandler.handleMessageToClient closing the session when it sees an ERROR frame over the clientOutboundChannel, isn't it?

[spring-projects-issues]

Rossen Stoyanchev commented

Yes but we do send the ERROR frame first. This is actually covered in the spec which says it's okay to close the connection just after sending the ERROR. See here. That means you may not get the ERROR frame.

[spring-projects-issues]

Gabriele Del Prete commented

Yes, I understand that.

I'm going to try with 4.2 to see if my problem disappears not that the CONNECT isn't swallowed anymore and it ultimately results in the connection being closed (with eventually an ERROR message coming back). Right now, with 4.1, if the broker is down and the browser issues a CONNECT message, the effect is that it gets stuck because it's wating for a response (CONNECTED) that never comes and the connection is kept alive by Spring.

[spring-projects-issues]

Rossen Stoyanchev commented

Resolving for now since the connection should be closed but would be great to confirm.

[spring-projects-issues]

Gabriele Del Prete commented

The follwing is using 4.2.RC3, SockJS-client 1.0.0. If the client is connected to Spring and spring is configured to relay to an external broker, as soon as the broker is shut down, an ERROR frame with message "Connection to broker closed." is sent. (see log line at 15:28:53.537). While the broker is stopped, the CONNECT message that the client sends when trying to reconnect (see 12:28:57.585) is swallowed, no ERROR message gets sent back to the client, and the client gets stuck forever (there is probably no timeout in SockJS-Client's handling of connection startup). Restarting the broker does not make any difference because the client is simply not issuing CONNECT messages anymore.

Having an ERROR message sent back would probably make SockJS-Client recognize something is wrong and at least give back control to the calling code.

angular.js:10103 >>> PING
2015-07-28 15:28:53.520 angular.js:10103 <<< PONG
2015-07-28 15:28:53.522 angular.js:10103 >>> PING
2015-07-28 15:28:53.537 angular.js:10103 <<< ERROR
message:Connection to broker closed.
content-length:0


2015-07-28 15:28:53.538 angular.js:10103 Received frame in errorCallback: Frame
2015-07-28 15:28:53.538 angular.js:10103 Received frame is for an ERROR message with message: "Connection to broker closed."
2015-07-28 15:28:53.539 angular.js:10103 Unknown error Connection to broker closed.
2015-07-28 15:28:53.540 angular.js:10103 Whoops! Lost connection to https://<somehost>:8443/app/push
2015-07-28 15:28:53.540 angular.js:10103 We got disconnected
2015-07-28 15:28:53.541 angular.js:10103 Disconnected
2015-07-28 15:28:57.542 angular.js:10103 Opening Web Socket...
2015-07-28 15:28:57.585 angular.js:10103 Web Socket Opened...
2015-07-28 15:28:57.585 angular.js:10103 >>> CONNECT
X-CSRF-TOKEN:0f25b905-1bca-4f2d-a281-cf2f8455a2c6
accept-version:1.1,1.0
heart-beat:10000,10000

[spring-projects-issues]

Rossen Stoyanchev commented

Strange, this here should ensure an ERROR frame is sent. Any idea what's happening if you put a break point there?

[spring-projects-issues]

Gabriele Del Prete commented

Sorry, just realized Eclipse did not get the new Spring version from Gradle and it was still running under ther 4.1.6 jars. I will get back to you as soon as I manage to have the application running (#17881 is blocking my migration now).

[spring-projects-issues]

Gabriele Del Prete commented

Confirmed working for 4.2.0.BUILD-SNAPSHOT for me too, thank you!

[spring-projects-issues]

Rossen Stoyanchev commented

Thanks.