xsf / xeps

Hosts the markup for all XMPP Protocol Extensions.
https://xmpp.org/extensions/
Other
125 stars 121 forks source link

XEP-0353: add disco (based upon PR #1141) #1142

Closed tmolitor-stud-tu closed 1 year ago

tmolitor-stud-tu commented 2 years ago

@jcbrand This is based upon my second PR (#1141) and adds a new revision (0.4.1)

This adds the disco feature suggested by Daniel Gultsch on the standards@ list.

tmolitor-stud-tu commented 2 years ago

@fippo @stpeter Please review this

fippo commented 2 years ago

sorry for the delay!

I don't think this is the way to go, the lack of a disco is intentional. In terms of UX you would probably timeout the attempt to use M-I and then proceed with falling back to the established behavior of discovering clients support 0166/0167 but that does not belong into this XEP and shouldn't use the disco feature.

tmolitor-stud-tu commented 2 years ago

The lack of disco already made (as far as I know) Conversations and others implement a disco even though it is not specified in the xep. What intentions does not having a disco fulfill?

@iNPUTmice could you clarify the usecases of having a disco a bit? First hand implementers experience could help a lot here.

tmolitor-stud-tu commented 2 years ago

As far as I know, both, Dino and Conversations implement a disco and I will implement one for Monal, too.

That makes this spec another spec that does not reflect reality, which is sad.

tmolitor-stud-tu commented 1 year ago

@fippo Why is the lack of disco intentional? Please elaborate!

fippo commented 1 year ago

The main use-cases of 0353 is sending a message to a bare JID in order for it to be forked to all available resources. This means you'd disco against the bare jid which typically (in the case of an IM server) won't support Jingle.

If you discover JMI support what do you gain? You'd still send to the bare JID and can't use the information returned from the information discovered from any available resource (and you can not make decisions about devices which appear to be offline but would get a push notification)

mwild1 commented 1 year ago

Related issue: https://github.com/snikket-im/snikket-server/issues/86

Based on the number of "why can't I see a call button?" support questions I see almost daily from a range of clients, I am inclined to lean towards less discovery and more graceful failure handling when JMI responds with an error or not at all.

That said, with the advent of long-lived device identifiers (new SASL2/bind2), I also consider this a candidate for a feature we could eventually advertise on the account rather than on individual devices (i.e. when the server knows a JMI-capable client has an active push registration).

tmolitor-stud-tu commented 1 year ago

@mwild1 Yes, I don't like Conversation's behavior here, too.

What I want to accomplish with Monal is: show a warning that the call might not reach the receiver if not at least one device can be found having that disco feature and don't show that warning at all, if at least one device can be found. I'm not going to hide the call button like Conversations does.

@fippo For that reason I'd appreciate to have a disco. I'm even going one step further: I will add such disco to Monal even if not specified in the XEP. But I'd like XEPs to reflect reality. And having at least Monal, Dino and Conversations publish such disco features makes me think that this should be added to the XEP as well.

And yes, surely with a big fat warning not to prevent calls if the disco can't be found because of temporarily offline devices that could still be woken up via push etc..

iNPUTmice commented 1 year ago

I am inclined to lean towards less discovery and more graceful failure handling when JMI responds with an error or not at all.

On paper I agree with you. However JMI will usually not respond with an error. It doesn’t even respond with an acknowledgment that some device has started ringing.¹ So if you have a perfectly good device online right now that supports direct call initialization but not JMI your timeout for going from JMI to let’s do direct IQ has to be fairly high, unnecessarily prolonging the time the device (that only supports direct init) starts ringing.

That’s what the disco feature in Siskin, Dino and Conversations is for.

In a world where JMI was a quasi requirement for jingle calls I would agree with you that always sending JMI is the right move. But this wasn’t the case in 2020 when Conversations implemented calls and with the namespace bump in JMI and competing XEPs that are dedicated for call initialization I’m not sure we will get there in the short term.

I think it's also somewhat ironic that Disco is a thing in XMPP and the one time when Conversations uses that to actually show or hide UI everyone hates it. In general I don’t think disco is the wrong move. We just need to figure out how to make disco work with offline devices (account disco?) instead of finding work arounds for each XEP that may want to rely on disco.

P.S.: Somewhat related (but not really). On one of the projects I work on we more or less got rid of JMI in favor of direct init for speed reasons. (less round trips; the bulk of the session information is already at the callers end when it starts ringing etc)

P.P.S.: I’m not even advocating for adding a disco feature. I’m just explaining why Conversations introduced it almost three years ago. If you consider supports old version of JMI as does not support JMI then Monal right now is in the exact same situation as Conversations was three years ago. Plenty of client support A/V calls. Virtually none of them support JMI. It would be a shame if Monal (or Conversations back then) artificially restricts what calls to call by making JMI a requirement.

¹: Conversations sneakily tries to fake this with delivery receipts.

tmolitor-stud-tu commented 1 year ago

@iNPUTmice Using jingle directly exhibits bad UX, especially on iOS where it can happen far more often that a client is offline (e.g. not even having a hibernated XEP-0198 session) making direct jingle fail. It is okay to do it as temporary workaround until JMI 0.4 support is more widespread.

But do you think, JMI 0.4 will be implemented by clients any time soon? Will Conversations implement it? If not, having direct jingle support in Monal wouldn't be a temporary workaround but a permanent "solution". A "solution" having very bad UX.

tmolitor-stud-tu commented 1 year ago

I think this can be closed now.

Many clients (at least Siskin, Dino, Conversations and Monal) will do disco. Either for timely fallback to direct call initialization or, like Monal, to at least show a note to the user that the JMI call might go unnoticed on the receivers end.

The XEP just does not reflect this reality, but that's fine by me.