Bug 46700

Summary: Implement reliable stanza delivery using XEP-0198 - Stream Management
Product: Telepathy Reporter: Michael Gratton <mike>
Component: gabbleAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED MOVED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: enhancement    
Priority: medium CC: accounts+bugs.freedesktop.org, daniel.scharon, development, devurandom, diane, eseifert, flo, flyser42, hugo, john.brooks, nickbroon, nirbheek.chauhan, nthn, paul, sam, scott, silverskullpsu, theodorstormgrade
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://xmpp.org/extensions/xep-0198.html
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 96921    
Attachments: stream management patch for wocky
enabling wocky-sm within gabble
all in one patch against git master
New Wocky patch against master
Gabble patch against master
New Wocky patch against master
basic XEP 198 unit-test

Description Michael Gratton 2012-02-27 14:54:42 UTC
Currently, intermittent network connections can cause gabble to regularly drop messages and other XMPP stanzas, both sent and received, since XMPP does not by default implement reliable stanza delivery. This is important since users have no feedback if a message has been successfully sent/received or not, and can result in social repercussions if a message is lost.

XEP-0198 adds an official way to implement reliable delivery, and at least jabberd2 implements it on the server side, thus gabble should support it, too.
Comment 1 Florian Jacob 2013-08-30 15:48:39 UTC
Because of the switch of Chakra to systemd and telepathy only supporting UPower suspend signals, after I suspend, I constantly loose all messages send to me in the next hours until the TCP Keepalive timer on the servers run out.

Even without suspend, lost connections happen frequently when you're online over mobile internet, which leads again to lost messages without any notice for anyone that the message is lost. This is really annoying.

In the current version, the prosody server supports XEP-0198, too (with mod_smacks loaded).


(Bug for systemd support is https://bugs.freedesktop.org/show_bug.cgi?id=68758 )
Comment 2 Simon McVittie 2013-09-02 11:57:10 UTC
See also <https://bugs.freedesktop.org/show_bug.cgi?id=41148#c3> for some thoughts about connectivity monitoring and how it relates to XEP-0198, although I'm not sure how much of that relates to suspend/resume - it might make sense for MC to centralize suspend/resume monitoring, and disconnect everything on suspend if it can, even if connectivity monitoring is delegated to each CM.

Basic ack-based XEP-0198 hopefully shouldn't be rocket science.

Stream resumption is likely to be more difficult, because a Telepathy Connection is stateless. Resuming within the same Connection without first signalling NetworkError would be a bit subtle but probably OK.

Resuming a previous Connection would require D-Bus API design: a new D-Bus signal from the Connection, emitted alongside ConnectionError just before DISCONNECTED, to tell MC "here is the state you'll need to reconnect me, please pass it back in next time" - including the sequence number of the last acknowledged client -> server stanza, the text of all subsequent stanzas, and the SM-ID. To have any possibility of not being XMPP-specific, MC would have to treat it as an arbitrary blob of uninterpreted D-Bus data structures. It could be passed back in to the CM as a hidden Parameter of type 'v', or some separate mechanism like "call this method before Connect()" (I think I prefer the latter, tbh).

I don't know where this goes on the boundary between Wocky and Gabble.
Comment 3 Jonathan Frederickson 2014-02-07 16:14:42 UTC
I feel this is an important feature for mobile clients using Telepathy, so I'm offering 0.20 BTC for implementation through FreedomSponsors: http://freedomsponsors.org/core/issue/442/
Comment 4 Dennis Schridde 2014-09-09 04:29:47 UTC
Any news?
Comment 5 Ferdinand Stehle 2014-10-23 20:11:35 UTC
Created attachment 108321 [details] [review]
stream management patch for wocky

This patch enables stream management within wocky and answers to acks requests. It will neither send requests by itself nor is resumption possible. 

At least the server will notice the broken connection :)
Comment 6 Ferdinand Stehle 2014-10-23 20:12:28 UTC
Created attachment 108322 [details]
enabling wocky-sm within gabble
Comment 7 Ferdinand Stehle 2014-10-26 16:34:57 UTC
Created attachment 108461 [details] [review]
all in one patch against git master

This patch only affects libwocky. 
It now sends requests, but still misses any feedback to the telepathy layer, therefore also to the user.
Comment 8 Ferdinand Stehle 2014-12-11 19:51:10 UTC
Created attachment 110749 [details] [review]
New Wocky patch against master
Comment 9 Ferdinand Stehle 2014-12-11 20:04:10 UTC
Created attachment 110753 [details] [review]
Gabble patch against master

Together with the latest wocky patch, gabble is able to report unacked stanzas when closing the connection.

We are still missing session resumption, but at least no messages should go missing without notification.
Comment 10 Ferdinand Stehle 2015-01-01 23:57:11 UTC
Created attachment 111628 [details]
New Wocky patch against master

Now containing the actual changes...
Comment 11 Thaodan 2015-01-02 15:45:20 UTC
Is there gui support of the frontend eg. telepathy-kde or empathy for instance, needed?
Comment 12 Ferdinand Stehle 2015-01-02 18:16:12 UTC
(In reply to Thaodan from comment #11)
> Is there gui support of the frontend eg. telepathy-kde or empathy for
> instance, needed?

When finally closing the connection, Empathy will show up a message like "message not sent". Unfortunately not next to the lost messages, but multiple times at the end of the log.
Comment 13 Thaodan 2015-01-03 01:14:44 UTC
So atleast emapthy needs support, what about other frontends like the sailfish or kde one?
Comment 14 Ferdinand Stehle 2015-01-03 17:52:15 UTC
(In reply to Thaodan from comment #13)
> So atleast emapthy needs support, what about other frontends like the
> sailfish or kde one?

I don't know, they need to handle TP_DELIVERY_STATUS_PERMANENTLY_FAILED. 

Even without user notification there are benefits in enabling stream management, as incoming message loss is detected by the server.
Comment 15 Ferdinand Stehle 2015-01-08 23:49:36 UTC
(In reply to Thaodan from comment #13)
> So atleast emapthy needs support [...]

See my lazy patch https://bugzilla.gnome.org/show_bug.cgi?id=742564
Comment 16 Ferdinand Stehle 2015-03-15 23:00:29 UTC
Three months and no feedback at all?

I modified the patchset for my Harmattan N9 and it works flawlessly with the stock messaging app which even displays failed delivery :)
Comment 17 daniel.scharon 2015-03-16 15:10:54 UTC
well, there are many who are eagerly waiting for your patchset to be reviewed and accepted ;-)
Comment 18 Ferdinand Stehle 2015-05-24 23:42:23 UTC
I published the branch on github, maybe someone wants to try and file some bugs:

https://github.com/noonien-d/telepathy-gabble/tree/XEP-0198_Stream_Management

https://github.com/noonien-d/wocky/tree/XEP-0198_Stream_Management
Comment 19 nthn 2015-08-11 16:39:48 UTC
Could someone _please_ take a look at this? Without this (and https://bugs.freedesktop.org/show_bug.cgi?id=23844 and https://bugs.freedesktop.org/show_bug.cgi?id=78093), Telepathy is essentially useless nowadays.
Comment 20 diane 2016-01-12 19:39:14 UTC
I've started reviewing Ferdinand Stehle's patches for this. They look like a promising start at providing the minimal XEP 198 support. Though I need to reread it again more carefully.

The code now passes the code style checks, but lacks its own unit tests.

We need test coverage before recommending merging.
Comment 21 diane 2016-01-19 07:32:27 UTC
Shouldn't the proper way of exposing if a message was acknowledged to use the delivery report functionality in http://telepathy.freedesktop.org/spec/Channel_Type_Text.html?

There's code in empathy and kde-telepathy to respond to delivery report messages.

However this patch currently doesn't emit those telepathy spec messages.
Comment 22 Florian Schmaus 2016-01-19 07:52:33 UTC
I don't beliebte so. XEP-0198 acks only acknowledge that the remote hop has processed the stanza. Delivery reports should likely be triggered by XEP-0184 receipts.
Comment 23 Dennis Schridde 2016-01-20 21:14:10 UTC
(In reply to Florian Schmaus from comment #22)
> I don't beliebte so. XEP-0198 acks only acknowledge that the remote hop has
> processed the stanza.

To prevent confusion: The *next hop*, i.e. the user's XMPP server.
Comment 24 diane 2016-01-20 22:21:36 UTC
Right. After reading Comment #22 I looked at XEP-184 and XEP-198 and yes. 198 is adding a new state, between message having been transmitted to the senders server before sending the message to the recipients server or having been delivered to the recipients client..

In trying to fit this new concept into the telepathy spec, I had two ideas. Either extend the definition of the Channel.Interfaces.MessageSent signal. Currently it says: 

  "This SHOULD be emitted as soon as the CM determines it's theoretically possible to send the message (e.g. the parameters are supported and correct)."

To me that implies the signal should be sent when the message was transmitted to server, but one could imagine waiting for acknowledgement that server received the message is a plausible interpretation.

My other idea was to add a new state to Delivery_Reporting_Support_Flags called something close to "Receive_Submission_Status".
Comment 25 Ferdinand Stehle 2016-01-20 22:42:35 UTC
(In reply to diane from comment #24)

MessageSent shouldn't be modified due to the implications on the telepathy-clients. Some won't even show the sent message in the log until the server replies an ack.

There is TP_DELIVERY_STATUS_ACCEPTED with exactly that meaning: "An intermediate server has accepted the message but the message has not been yet delivered to the ultimate recipient." [1]

[1] http://telepathy.freedesktop.org/doc/telepathy-glib/telepathy-glib-enums.html
Comment 26 diane 2016-01-20 23:04:21 UTC
I skimmed right over TP_DELIVERY_ACCEPTED, yes that looks right.

So we have TP_DELIVERY_STATUS_ACCEPTED when the message is acknowledged. 

Currently the patch doesn't do anything when the connection is lost and there are unacknowledged stanzas. 

see: https://github.com/noonien-d/wocky/blob/XEP-0198_Stream_Management/wocky/wocky-c2s-porter.c#L1260

A first implementation would be to fix that test to eventually send a Delivery_Status::Permanently_Failed notification to the client?
Comment 27 Ferdinand Stehle 2016-01-20 23:12:19 UTC
(In reply to diane from comment #26)

That part is already located in gabble, see

  src/im-factory.c:460 gabble_im_factory_report_unacked
Comment 28 diane 2016-02-07 05:17:49 UTC
(In reply to Ferdinand Stehle from comment #27)
> (In reply to diane from comment #26)
> 
> That part is already located in gabble, see
> 
>   src/im-factory.c:460 gabble_im_factory_report_unacked

Ah, though if stream management is available shouldn't TP_DELIVERY_STATUS_ACCEPTED be emitted only after receiving the acknowledgement?

I see a call in src/im-factory.c:580 (XEP-0198_Stream_Management branch).
but the surrounding code suggests thats its just always emitted for ones own messages.

Also it would be odd if we generate an _ACCEPTED message and then later send a _FAILED message. (I'm not sure if that happens yet, but it currently seems possible)
Comment 29 diane 2016-02-07 05:31:28 UTC
Created attachment 121565 [details]
basic XEP 198 unit-test

This is a unit test patch for Ferdinand Stehle's XEP-0198 wocky patch. 

It contains two git format-patch patches.

One is a simple do the provided getters & setters work.

The other implements enough of the negotiation in the unit test xmpp server to verification that it XEP-0198 is negotiated correctly. It could stand to have some of the invalid cases in the XEP-0198 negotiation tested, as well as testing to make sure the <a/> and <r/> stanzas are exchanged properly.

It depends on him merging a rename in his github branch.
https://github.com/noonien-d/wocky/pull/2

In order to run my test I had to fix several other tests.

https://bugs.freedesktop.org/show_bug.cgi?id=79548
https://bugs.freedesktop.org/show_bug.cgi?id=94031
https://bugs.freedesktop.org/show_bug.cgi?id=94032

There's still one failing test: /connector+ssl/econnreset/server-open unrelated to the XEP-0198 changes.
Comment 30 diane 2016-06-04 18:29:44 UTC
So I believe Ferdinand's patches provide the minimal XEP-198 support as described in http://xmpp.org/extensions/xep-0198.html#scenarios-basic

Should we commit that? Or hold out for the stream resumption so we can cut down on presence retransmission?
Comment 31 GitLab Migration User 2019-12-03 19:56:20 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-gabble/issues/217.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.