Add support for downstream labeled-response

delthas commented

2025-05-18 17:25:52 +02:00

Collaborator

Moved over from: https://github.com/emersion/soju/pull/40

Rebased since May 2022! 💪😭

This adds support for downstream labeled response. Instead of doing
specific processing on each SendMessage call site, the idea is to add
some logic to SendMessage.

The cap is advertised only in single-upstream mode when the upstream
server supports it too.

When handling a downstream message, we store the label of the current
message we're processing, in the downstream struct. (This should be
understood as data whose lifetime is limited to the processing of one
message. It is reset when we're done handling the message.)

During the handling of that downstream message, at any point, when we
send out messages to this downstream:

if we're not handling a particular label, send it as is
else if this is the first message we're sending, store/buffer it
else, open a batch, send/flush the pending message and our message in
that batch.

Then, at the end of the processing:

if we're not handling a particular label, do nothing
if we have a pending message, send it without a batch
if we had opened a batch, close it
if we didnt send anything, send ACK

This enables us to always send the minimum amount of messages.

2+ messages generates a BATCH
1 message generates a message with label
0 message generates an ACK with label

While we may send messages to many upstreams and downstreams, obviosuly
only the downstream from which the message we're processing was received
is affected.

A special case affects the above behavior, which makes it differ from a
typical server implementation. Soju handles many commands by forwarding
the command to the server, ending the downstream message handling
without responding anything, then forwarding the server response later
when we receive it.

In this case we must not send an empty ACK at the end of our routine,
but we must instead not send anything, copy the message label to the
message we're sending to the server, and then when we get the response
(or batches) from the server, forward that to the downstream.

Examples:

dc->soju: @label=foo BAR
soju->uc: @label=foo BAR
uc->soju: @label=foo BAZ
soju->dc: @label=foo BAZ

dc->soju: @label=foo BAR
soju->uc: @label=foo BAR
uc->soju: @label=foo BATCH +b labeled-response
soju->dc: @label=foo BATCH +b labeled-response
uc->soju: @batch=b BAZ
soju->dc: @batch=b BAZ
uc->soju: BATCH -b
soju->dc: BATCH -b

(This is a bit more complicated because we wrap downstream labels with
our own labels.)

The patch therefore adds a DeferredResponse method to downstreamConn,
which basically tells it not to send an empty ACK at the end of the
routine, because we'll do a proper response later.

When handling upstream messages, we extract the downstream label from
the server labeled response, if it exists. We also extract the
downstream batch tag if it corresponds to a label. We store that info in
the downstreamConn, much like the downstream message handling.

We forward BATCH + and BATCH - as is, even keeping the same batch IDs.

This way, we also support cases where the server responds with a single
message but our logic turns that into multiple messages, or no messages.
(We'd buffer the first message, open a batch, send the messages, close
it etc. just like the downstream message case.)

Some cases are a bit more difficult to implement and not really useful.
(And also not mandatory according to the spec since this is
best-effort.) Those were left TODO for now.

Moved over from: https://github.com/emersion/soju/pull/40 Rebased since May 2022! 💪😭 ---- This adds support for downstream labeled response. Instead of doing specific processing on each SendMessage call site, the idea is to add some logic to SendMessage. The cap is advertised only in single-upstream mode when the upstream server supports it too. ---- When handling a downstream message, we store the label of the current message we're processing, in the downstream struct. (This should be understood as data whose lifetime is limited to the processing of one message. It is reset when we're done handling the message.) During the handling of that downstream message, at any point, when we send out messages to this downstream: - if we're not handling a particular label, send it as is - else if this is the first message we're sending, store/buffer it - else, open a batch, send/flush the pending message and our message in that batch. Then, at the end of the processing: - if we're not handling a particular label, do nothing - if we have a pending message, send it without a batch - if we had opened a batch, close it - if we didnt send anything, send ACK This enables us to always send the minimum amount of messages. - 2+ messages generates a BATCH - 1 message generates a message with label - 0 message generates an ACK with label While we may send messages to many upstreams and downstreams, obviosuly only the downstream from which the message we're processing was received is affected. ---- A special case affects the above behavior, which makes it differ from a typical server implementation. Soju handles many commands by forwarding the command to the server, ending the downstream message handling without responding anything, then forwarding the server response later when we receive it. In this case we must not send an empty ACK at the end of our routine, but we must instead not send anything, copy the message label to the message we're sending to the server, and then when we get the response (or batches) from the server, forward that to the downstream. Examples: dc->soju: @label=foo BAR soju->uc: @label=foo BAR uc->soju: @label=foo BAZ soju->dc: @label=foo BAZ dc->soju: @label=foo BAR soju->uc: @label=foo BAR uc->soju: @label=foo BATCH +b labeled-response soju->dc: @label=foo BATCH +b labeled-response uc->soju: @batch=b BAZ soju->dc: @batch=b BAZ uc->soju: BATCH -b soju->dc: BATCH -b (This is a bit more complicated because we wrap downstream labels with our own labels.) The patch therefore adds a DeferredResponse method to downstreamConn, which basically tells it not to send an empty ACK at the end of the routine, because we'll do a proper response later. ---- When handling upstream messages, we extract the downstream label from the server labeled response, if it exists. We also extract the downstream batch tag if it corresponds to a label. We store that info in the downstreamConn, much like the downstream message handling. We forward BATCH + and BATCH - as is, even keeping the same batch IDs. This way, we also support cases where the server responds with a single message but our logic turns that into multiple messages, or no messages. (We'd buffer the first message, open a batch, send the messages, close it etc. just like the downstream message case.) ---- Some cases are a bit more difficult to implement and not really useful. (And also not mandatory according to the spec since this is best-effort.) Those were left TODO for now.

emersion force-pushed feature-labeled-response from 7507260f9c

builds.sr.ht Job completed

Details

to 1d7c27131d

builds.sr.ht Job completed

Details

2025-07-24 18:18:32 +02:00

Compare

emersion commented

2025-07-24 18:20:12 +02:00

Owner

I've rebased this patch against master.

emersion referenced this pull request

2025-07-24 18:20:54 +02:00

Support METADATA for users #54

emersion force-pushed feature-labeled-response from 1d7c27131d

builds.sr.ht Job completed

Details

to 7b8a99cad4

builds.sr.ht Job completed

Details

2025-07-25 16:23:53 +02:00

Compare

emersion commented

2025-07-25 16:49:53 +02:00

Owner

I've pushed the ircError handling change as 967cb94695, because it had value on its own.

I've pushed the `ircError` handling change as 967cb94695a91f8dfac01e174f262bd5431b5296, because it had value on its own.

emersion force-pushed feature-labeled-response from 7b8a99cad4

builds.sr.ht Job completed

Details

to da1f7af720

builds.sr.ht Job completed

Details

2025-07-25 16:51:40 +02:00

Compare

emersion commented

2025-07-25 17:38:21 +02:00

Owner

This is what I had in mind regarding context use over stuffing temporary fields into downstreamConn: https://codeberg.org/emersion/soju/compare/master...downstream-labeled-response

Do you see an issue with this approach?

This is what I had in mind regarding context use over stuffing temporary fields into `downstreamConn`: https://codeberg.org/emersion/soju/compare/master...downstream-labeled-response Do you see an issue with this approach?

delthas force-pushed feature-labeled-response from da1f7af720

builds.sr.ht Job completed

Details

to 7569b3c7da

builds.sr.ht Job completed

Details

2025-09-24 15:08:27 +02:00

Compare

delthas commented

2025-09-24 15:08:50 +02:00

Author

Collaborator