[1.x] Batch requests fully fail even if one of the feed ids is not valid / non-existing

[petarminchev]

When somebody joins a video conference or doing paging, we do a batch subscribe/unsubscribe to a list of feed id / mids. This is more efficient than doing separate subscription request for each feed and mid.

But there is a caveat/race condition, that one of the publishers can leave the room while the batch subscription is initiated / in-progress. That causes Janus to reject the whole batch subscription with a no such feed id error.

https://github.com/meetecho/janus-gateway/blob/master/src/plugins/janus_videoroom.c#L10338

In our opinion Janus should still subscribe / unsubscribe to/from the valid feed ids, and for the invalid ones the error could still be thrown separately, but they shouldn't block the valid ones. Using "continue" instead of "goto error".

Without that change the only workaround possible on the client side is to retry without the invalid feed id again when Janus returns the error. It could get a little annoying if there are 3-4 such feed ids, cause we would need to retry 3 or 4 times, since Janus returns only the first invalid id.

What do you think?

[lminiero]

I'm not sure what should be done, but it sounds like a breaking change, considering there might be people relying on the error instead. I'm personally not fond of this continue approach, especially if it results in a silent execution of the rest: you may expect some things to have happened, and instead things didn't happen unbeknownst to you, maybe because you did a typo.

A possible approach might be:

• by default, keep doing what we do (missing ID = error) to ensure backwards compatibility
• add option to be more tolerant, and skip the invalid IDs

A list of skipped IDs might be nice to have, to explicitly inform the caller about what was ignored, but we may have different events depending on the state: in fact, in some cases we'll send an updated (possibly with a new SDP) right away, while in other cases we'll send an updating (e.g., when still waiting from a previous renegotiation).

I think this is only apparently an easy fix, though, since our pre-checks are also used to sanitize the input to the code that actually goes through the changes. I'm at the IETF meeting now, so not sure when I'd be able to work on this.

[petarminchev]

Thanks for the response! Adding an option flag for tolerance sounds like a good approach to me. And as you have said you already iterate to find the invalid IDs.

[petarminchev]

Is there a chance implementation of this will get prioritized in January? Asking so we can know if to implement the retries as a temporary solution.

[lminiero]

Not sure yet if it will happen in January, sorry.