Allow sending custom headers in more frames

[aruke]

Problem / use case

I ran into a use case where I would like to send custom headers in an ACK frame. This might sound unusual, given the specification only mentions the headers id and transaction. However, I'm porting a JS implementation, and the JS client allows arbitrary headers. This might be because of loose typing from JS/TS, but sending headers works.

Feature (solution)

This is not a standard requirement, so if the library allows sending raw STOMP frames from the session, it will be more flexible.
One solution can be adding a rawFrame(frame: StompFrame) in the StompSession interface. There is already such implementation, but it's a private method.

Current alternatives

I haven't found any workarounds yet. If there is one, I would like to go with it instead of requesting library changes.

[joffrey-bion]

Hello! Thanks for sharing your use case. I intend to do some rework around headers in general at some point, and this is good input.

Indeed the spec technically only allows user-defined headers in MESSAGE, SEND, and ERROR frames, and that's why Krossbow doesn't currently allow sending custom headers in other frames.
That said, it doesn't really hurt to be lenient in the client and allow extra headers in any frame, possibly protected by an opt-in annotation for frames that shouldn't support it.

Sending a raw StompFrame is currently internal on purpose, because the StompSession needs to do some special handling for subscriptions and receipts and sending single frames generically might cause problems.
It's not necessary to make this public in order to solve your issue, but maybe it would be a more general way to solve a whole class of problems around deviating from the spec. An opt-in annotation would be sufficient to discourage misuses.

[aruke]

It doesn't really hurt to be lenient in the client and allow extra headers in any frame, possibly protected by an opt-in annotation for frames that shouldn't support it.

This would have been an ideal way to deal with custom headers, but not only it deviates the library from specifications, but also adds extra code to be tested.

The StompSession needs to do some special handling for subscriptions and receipts and sending single frames generically might cause problems.

I understand, but this adds flexibility to whoever wants to use the library for custom needs.

The current problem with making sendStompFrame public is that the HeartBeater will not be notified with the public version of sendStompFrame.
At the same time, I realized that even making sendStompFrame won't solve custom header problem, because, let's say I want to send custom headers for RECEIPT frame, I cannot because:

StompFrame.Receipt(StompReceiptHeaders(StompHeaders())) // StompHeaders is interface
StompFrame.Receipt(StompReceiptHeaders(SimpleStompHeaders())) // SimpleStompHeaders is internal

With this the first option starts to look better.

[joffrey-bion]

Yes, I enforced the spec constraints in multiple places :) That's why I will need to change things around the headers to make this happen. But anyway headers need to change. I don't like the current mutability. It was done as a premature performance optimization, but it has a cost in correctness.

I'll keep this issue in mind when I do rework this part.

And also I'll see what's best for sendStompFrame.

[aruke]

Meanwhile, is there a workaround for sending custom headers without writing my own WebsocketClient?

[joffrey-bion]

I tried to look for one, but I don't think there is, unfortunately. Sorry

[aruke]

I tried making changes in this PR the header constructors. Can you please let me know if anything more is needed regarding code or unit tests?

[joffrey-bion]

It turns out the spec actually opens the door for anything in this sentence:

Finally, STOMP servers MAY use additional headers to give access to features like persistency or expiration. Consult
your server's documentation for details.

So I decided to allow custom headers without opt-in in the new headers design.