Add handleErrorFrame callback for STOMP handler

[ppshinebl]

The previous implementation invoke handleFrame no matter the frame is "MESSAGE" nor "ERROR". This commit add a new callback "handleErrorFrame" to the handler to separate from the message frame handling.

Reason:

1. The error frame is created by server and not following bussiness message frame format.
2. Otherwise the handler can not tell if the frame is business message or it's an error message.

[pivotal-issuemaster]

@ppshinebl Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@ppshinebl Thank you for signing the Contributor License Agreement!

[cuspymd]

It seems that the year needs to be updated in the license header of the modified files.
Please refere License section in Code-Style.

[cuspymd]

documentation seems be needed.

[rstoyanchev]

To handle an ERROR frame, use the global StompSessionHandler that you provide to the connect method. That's the only frame it will receive because MESSAGE frames have a subscription header and are passed to the StompFrameHandler for the subscription that you provide via StompSession#subscribe. Such subscription specific StompFrameHandler instances will never see an ERROR frame and it would be misleading to expose the method in StompFrameHandler.

If anything it would be an option to expose handleErrorFrame in StompSessionHandler but considering it already inherits handleFrame and also that ERROR is the only frame that can be received at that level, it seems unnecessary to create a separate method. It is also not backwards compatible.

Thanks for the suggestion in any case.

[ppshinebl]

Hi @rstoyanchev

1. According to STOMP1.2 spec

If the server cannot successfully create the subscription, the server MUST send the client an ERROR frame and then close the connection.

The ERROR frame could be subscription specific.

2. As a mention in the merge request as following, headers are filtered and raw headers are not passed to the StompFrameHandler, "command" type is also invisible, so it's necessary to create a new method to handle the ERROR frame.

The error frame is created by server and not following bussiness message frame format.

Otherwise`` the handler can not tell if the frame is business message or it's an error message.

I agree that it would make more sense to move handleErrorFrame from StompFrameHandler to StompSessionHandler, do you have any more concern?

[royclarkson]

@ppshinebl did you mean to mention @rstoyanchev on your previous comment?

[rstoyanchev]

The ERROR frame could be subscription specific.

I think you mean it can be related or triggered by but the ERROR frame is not linked to a subscription in any formal way. It would have to have the subscription id, just like the MESSAGE frame does but it doesn't. Furthermore, the connection is closed immediately after. That means that only 1 ERROR frame can ever be received globally, not linked to a subscription.

I don't think it makes sense to add a dedicated handleErrorFrame method. Use the handleFrame on the StompSessionHandler and that will be the only frame it can receive. There is no ambiguity there.

[ppshinebl]

I don't think it makes sense to add a dedicated handleErrorFrame method. Use the handleFrame on the StompSessionHandler and that will be the only frame it can receive. There is no ambiguity there.

The STOMP specification defines an ERROR frame, but in the handler just mix it with normal message frame handling, that's misleading and very amibigous.
Also when handling both message frame and error frame, how can tell if it is normal message or an error? If the message context type is json, and the error frame format is defined by broker, how to deserialize both format type with limitted header?
For current implementation, just trying to format the error frame like message frame using the same and throwing exceptions, I think that's just a work around solution.

[rstoyanchev]

No there is no mixing. The global StompSessionHandler does not get any frames.

[ppshinebl]

@rstoyanchev
Any comment?