All of the _OK and _ERROR messages are annoying and redundant

[afrind]

As I wrote up the PR for SUBSCRIBE_NAMESPACE, I found it kind of annoying that I had to add these messages, when they are largely boilerplate copies of other messages. If we get to 3 or more messages that have _OK and _ERROR, it's probably time to refactor?

I'm thinking we can make generic CONTROL_OK and CONTROL_ERROR messages that satisfy the basics, and possibly extend them when we have additional message-specific fields.

One rough sketch would be that every control message has a Control Message ID (which is a superset of the existing subscribe ID), and the new messages are:

CONTROL_OK {
  Control Message ID (i)
  Type (i)
  [Control OK Payload (..)]
}

CONTROL_ERROR {
  Control Message ID (i)
  Error Code (i)
  Reason Phrase (b)
  Type (i)
  [Control Error Payload (..)]
}

The Type field would contain the code point for the original control message, and the payload would be present if needed (eg: SUBSCRIBE_OK). Or maybe the Control Message ID space reuses the code-point bits to encode the message type, or something.

[ianswett]

I tend to agree, want to write a PR?

[afrind]

I think we need a little bit of discussion to flesh out what a PR would look like:

1. What do people think of assigning a control-message ID to control messages that have a response (SUBSCRIBE, ANNOUNCE, SUBSCRIBE_NAMESPACE)?
2. Should we merge subscribe ID completely into this space? What about MAX_SUBSCRIBE_ID? If we change this to MAX_CONTROL_ID, then one has to leave headroom for non-subscribe control messages. If not, then SUBSCRIBE has both Control Message ID and Subscribe ID?
3. Do we like a separate varint for Type as shown, or do we want to collapse this into the control message type (this is starting to not look much different from what's there now)?

[suhasHere]

Slightly different proposal would be to have pairs of messages

1. Subscribe, Subscribe Response
2. Announce, Announce Response
3. SubscribeNamespace, SubscibeNamespceResponse

The response message identifies if it an error/ok and any contextual payloads. this approach keeps each message semantics to its req/resp pairs rather than generic Union Type like Control message

Thoughts ?

[martinduke]

You could put the message type in the response to avoid having a control message ID

[ianswett]

Looking at all the messages, there are some I'm unsure if we even need, like ANNOUNCE_OK and ANNOUNCE_ERROR?

QUIC Streams are reliable, so you know the peer got the ANNOUNCE. ANNOUNCE_CANCEL is also a bit unusual. Does the publisher need to know you're not going to send subscriptions for that namespace to it anymore?

As we've discussed, the use of SUBSCRIBE_DONE is also a bit unclear today.

So I'm inclined to wait on this, but I agree it's a bit ugly today.

[kixcord]

I agree, there are too many redundant OK/ERROR/CANCEL/UN messages.

Transfork is using a QUIC bidirectional stream for each ANNOUNCE and SUBSCRIBE. Either side can reset the stream (with an error code) to cancel the transaction. This replaces:

• ANNOUNCE_ERROR
• ANNOUNCE_DONE
• ANNOUNCE_CANCEL
• UNANNOUNCE
• SUBSCRIBE_OK
• SUBSCRIBE_ERROR
• SUBSCRIBE_DONE
• SUBSCRIBE_CANCEL
• UNSUBSCRIBE

I still have ANNOUNCE_OK because it's useful to know when the relay has accepted the announce (ex. 200 OK). SUBSCRIBE_OK has been replaced with TRACK_INFO.

[kixcord]

If a QUIC bidirectional stream for each action is too "expensive", then you could use a Transaction ID in place of the Stream ID to associate requests with responses.

[fluffy]

I don't want both a subscribe_ID and control_ID. If we move to a generic control_id, it should replace subscribe_ID.

[fluffy]

All of this seems not all that important to solve right now and that it will raise a lot of issues. For example, we have not added in auth yet but that might tend to make the messages look not as common as they are today. It might be a good plan to get the auth stuff semi figured out before deciding how common these messages are or not.