ERROR or RECEIPT

[sywka]

Description

When I send an error on the server for any frame with a receipt header set, I get both the RECEIPT and the ERROR messages on the client.

From specification:

If the sever cannot successfully process the SEND frame for any reason, the server MUST send the client an ERROR frame and then close the connection.

and

If the error is related to a specific frame sent from the client, the server SHOULD add additional headers to help identify the original frame that caused the error

As I understand it, in this case ERROR acts as a confirmation and RECEIPT should not be sent.

Versions

node-stomp-protocol: 0.3.8

[pcan]

I have a fix ready for this one, but there is an interesting point that deserves attention:

If the error is related to a specific frame sent from the client, the server SHOULD add additional headers to help identify the original frame that caused the error

currently there is no way to implement the bold part inside the protocol, since the call to the command handler (see StompServerSessionLayer.handleFrame) may not be synchronous with the ERROR frame you are sending as reply (please note that StompServerSessionLayer.error is async). This implies your SEND handler should take care of this issue, if you need the same receipt-id. If you don't want to care of this, you may simply throw an Error inside your handler (if it's synchronous code), and the handleFrame method will take care of the receipt-id (see should send ERROR with receipt when catching exceptions from listener test). Let me know what you think.

[sywka]

I've already looked at the source code and in my case it's enough throw an Error inside the handler, but I was confused by the moment with an asynchronous error.

It seems to me that the server must respond RECEIPT or ERROR only after the message has been completely processed (including asynchronous handlers). Based on this, there are two options:

1. hide the solution to this problem in the implementation of the protocol (it's also hide the use of the message "ERROR" and "RECEIPT" from the public api)
   • async handlers with Promise.
     It will be necessary to solve the problem Calling async function from sync function leads to surprising behavior #6 in another way. To send the error will be used throw new StompError(headers, body) inside handler
   • async handlers without Promise.
     Implement the mechanism of middlewares like express
     To send the error will be used callback function return done(new StompError(headers, body)) inside handler
2. simply delete the automatic sending RECEIPT from the implementation protocol and shift it to the user

[pcan]

I think the project needs to move toward point two, for a simple reason: the goal is not to build a broker (as stated in the README), so the library should not handle specific behaviour that may be left to the user. But now another question arises: should we do the same thing for transactions? at the moment, the library keeps an object in memory containing all pending transactions for each session (see here) and we have the same synchronization issue similar to the RECEIPT.

[sywka]

I think if we stick to the second point, then the library should not have the storage of transactions and subscriptions

[pcan]

I'm reviewing the code right now, and I think that we could achieve both a simpler protocol handler and a high-level broker-like protocol handler, if the library keeps these two concerns wisely separated. I think I'm going to remove all broker-specific checks from protocol.ts and session.ts; then I'm going to implement another class that provides an high-level API, and I think the solution you proposed about the done function is the best one, since it's more intention-revealing (maybe, this is the reason why express.js does things in this way).