stomp-js / rx-stomp

STOMP adaptor for RxJS
Apache License 2.0
111 stars 21 forks source link

Add correlateErrors feature #470

Closed vigie closed 1 year ago

vigie commented 1 year ago

Summary

This commit introduces a new, non-breaking feature: error correlation. Destination-specific errors can now be received on the corresponding destination observables as terminal errors.

The user can provide a callback called correlateErrors. This callback is called against every error frame. If the callback returns the ID of a currently subscribed destination, the error frame is emitted as an error on the corresponding observables, terminating them. In this manner, better use of the native observable API is made for clients working against brokers that may issue destination-specific errors. Also, we ensure a disconnect-reconnect loop is avoided by preventing the automatic re-subscription of a destination that produced an error.

Motivation

Some brokers may wish to produce destination-specific errors. For example, a broker might check the users token on SUBSCRIBE to a particular destination for correct destination-specific privileges, sending an ERROR if the required privileges are absent. In this case, the STOMP spec advises the server to mention the offending destination ID somewhere in the frame, without mandating exactly how (https://stomp.github.io/stomp-specification-1.2.html#ERROR).

Without this error correlation feature, the error is only emitted from a general purpose stompErrors$ observable and the error callbacks of the related observables are unused. This means that the related observable remains open, causing a re-SUBSCRIBE to be issued to its destination when the socket automatically re-connects. This can result in a tight loop of error-drop-reconnect, effectively bricking the entire socket for all channels.

Additional

kum-deepak commented 1 year ago

Many thanks for this PR. It is a good improvement and I will definitely merge it.

I have some other minor changes planned, I will combine those and release 1.2

I will need a couple of days to review, accordingly, I will provide feedback or go ahead and merge it.

vigie commented 1 year ago

@kum-deepak gentle bump.... Have you had a chance to review?

kum-deepak commented 1 year ago

I am sorry, I have been traveling with limited connectivity. Please allow a few more days.

kum-deepak commented 1 year ago

Many thanks fot this. I have reviewed this. I will merge and release this as part of the next release,

I guess you have used it with a broker, which broker and what is the correlateErrors function that worked?

I will love to include that in the documentation.

vigie commented 1 year ago

Many thanks fot this. I have reviewed this. I will merge and release this as part of the next release,

I guess you have used it with a broker, which broker and what is the correlateErrors function that worked?

I will love to include that in the documentation.

Thanks for your help!

At work we have a rabbitmq broker that will now add a custom destination header for any error that is destination specific. So, after I upgrade to the next rx-stomp release, I will be configuring it with a correlateErrors function that is simply

        correlateErrors: (errorFrame: IFrame) => {
          return errorFrame.headers.destination;
        }
vigie commented 1 year ago

@kum-deepak just for my planning, when do you expect to release next version? Thanks.