Summary: | Implement reliable stanza delivery using XEP-0198 - Stream Management | ||
---|---|---|---|
Product: | Telepathy | Reporter: | Michael Gratton <mike> |
Component: | gabble | Assignee: | 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
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 ) 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. 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/ Any news? 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 :) Created attachment 108322 [details]
enabling wocky-sm within gabble
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. Created attachment 110749 [details] [review] New Wocky patch against master 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. Created attachment 111628 [details]
New Wocky patch against master
Now containing the actual changes...
Is there gui support of the frontend eg. telepathy-kde or empathy for instance, needed? (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. So atleast emapthy needs support, what about other frontends like the sailfish or kde one? (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. (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 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 :) well, there are many who are eagerly waiting for your patchset to be reviewed and accepted ;-) 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 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. 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. 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. 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. (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. 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". (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 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? (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 (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) 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. 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? -- 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.