Attachment 56438 Details for Bug 45491 – Presence: show better error status messages.

[patch] Presence: show better error status messages.

0001-Presence-show-better-error-status-messages.patch (text/plain), 5.52 KB, created by Will Thompson on 2012-02-01 03:58:17 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Will Thompson

Created: 2012-02-01 03:58:17 UTC

Size: 5.52 KB

patch

obsolete

>From ec7b2fe3e65cb457b07ad7b53d8818876af36b7b Mon Sep 17 00:00:00 2001
>From: Will Thompson <will.thompson@collabora.co.uk>
>Date: Wed, 1 Feb 2012 11:16:51 +0000
>Subject: [PATCH] Presence: show better error status messages.
>
>When you get <presence type='error'>, any <status/> element in the
>presence is an echo of a stanza you sent out. So the message in that is
>not the contact's presence: it's either your status, or maybe the
>message you sent with a subscription request, depending on whether we
>got the <presence type='error'/> for a roster contact or someone we
>tried to subscribe to.
>
>So instead, I think we should use the text of the error (if any) as the
>status for such contacts, falling back to the error element name if the
>server is unkind and doesn't include an error message. Yes, this means
>strings will show up in the UI in the server's locale, but this is
>hardly news.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=45491
>---
> src/presence-cache.c            |   19 ++++++++++
> tests/twisted/Makefile.am       |    1 +
> tests/twisted/presence/error.py |   71 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 0 deletions(-)
> create mode 100644 tests/twisted/presence/error.py
>
>diff --git a/src/presence-cache.c b/src/presence-cache.c
>index bf5f87b..4b73104 100644
>--- a/src/presence-cache.c
>+++ b/src/presence-cache.c
>@@ -43,6 +43,7 @@
> #include <wocky/wocky-utils.h>
> #include <wocky/wocky-namespaces.h>
> #include <wocky/wocky-data-form.h>
>+#include <wocky/wocky-xmpp-error-enumtypes.h>
> 
> #define DEBUG_FLAG GABBLE_DEBUG_PRESENCE
> 
>@@ -1837,11 +1838,29 @@ gabble_presence_parse_presence_message (
>       return TRUE;
> 
>     case WOCKY_STANZA_SUB_TYPE_ERROR:
>+    {
>+      GError *error = NULL;
>+      gboolean ret;
>+
>       NODE_DEBUG (presence_node, "Received error presence");
>+
>+      ret = wocky_stanza_extract_errors (message, NULL, &error, NULL, NULL);
>+      g_assert (ret);
>+
>+      /* If there's a <status/> in this presence, it's our own echoed back at
>+       * us. So we don't want to use that. Instead, we use the <error><text> if
>+       * there is any, or the name of the error condition if not. */
>+      if (tp_str_empty (error->message))
>+        status_message = wocky_enum_to_nick (WOCKY_TYPE_XMPP_ERROR,
>+            error->code);
>+      else
>+        status_message = error->message;
>+
>       gabble_presence_cache_update (cache, handle, resource,
>           GABBLE_PRESENCE_ERROR, status_message, priority);
> 
>       return TRUE;
>+    }
> 
>     case WOCKY_STANZA_SUB_TYPE_UNAVAILABLE:
>       if (gabble_roster_handle_sends_presence_to_us (priv->conn->roster,
>diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
>index 7c756ff..03638bb 100644
>--- a/tests/twisted/Makefile.am
>+++ b/tests/twisted/Makefile.am
>@@ -64,6 +64,7 @@ TWISTED_TESTS = \
> 	plugin-channel-managers.py \
> 	power-save.py \
> 	presence/decloak.py \
>+	presence/error.py \
> 	presence/initial-contact-presence.py \
> 	presence/initial-presence.py \
> 	presence/invisible_xep_0126.py \
>diff --git a/tests/twisted/presence/error.py b/tests/twisted/presence/error.py
>new file mode 100644
>index 0000000..53f3703
>--- /dev/null
>+++ b/tests/twisted/presence/error.py
>@@ -0,0 +1,71 @@
>+"""
>+Tests that Gabble provides at least some useful information from error
>+presences.
>+"""
>+
>+from gabbletest import exec_test, make_presence, elem
>+from servicetest import assertEquals
>+import ns
>+import constants as cs
>+
>+def test(q, bus, conn, stream):
>+    jids = ['gregory@unreachable.example.com',
>+            'thehawk@unreachable.example.net',
>+           ]
>+    gregory, hawk = jids
>+    gregory_handle, hawk_handle = conn.RequestHandles(cs.HT_CONTACT, jids)
>+
>+    event = q.expect('stream-iq', query_ns=ns.ROSTER)
>+    event.stanza['type'] = 'result'
>+    for jid in jids:
>+        item = event.query.addElement('item')
>+        item['jid'] = jid
>+        item['subscription'] = 'both'
>+
>+    stream.send(event.stanza)
>+    q.expect('dbus-signal', signal='PresencesChanged',
>+        args=[{gregory_handle: (cs.PRESENCE_OFFLINE, 'offline', ''),
>+               hawk_handle:    (cs.PRESENCE_OFFLINE, 'offline', ''),
>+              }
>+             ])
>+
>+    # Our server can't resolve unreachable.example.com so it sends us an error
>+    # presence for Gregory. (This is what Prosody actually does.)
>+    presence = make_presence(gregory, type='error')
>+    error_text = u'Connection failed: DNS resolution failed'
>+    presence.addChild(
>+        elem('error', type='cancel')(
>+          elem(ns.STANZA, 'remote-server-not-found'),
>+          elem(ns.STANZA, 'text')(
>+            error_text
>+          )
>+        ))
>+
>+    stream.send(presence)
>+
>+    e = q.expect('dbus-signal', signal='PresencesChanged')
>+    presences, = e.args
>+    type_, status, message = presences[gregory_handle]
>+    assertEquals(cs.PRESENCE_ERROR, type_)
>+    assertEquals('error', status)
>+    assertEquals(error_text, message)
>+
>+    # How about maybe the hawk's server is busted?
>+    presence = make_presence(hawk, type='error')
>+    presence.addChild(
>+        elem('error', type='cancel')(
>+          elem(ns.STANZA, 'internal-server-error'),
>+        ))
>+    stream.send(presence)
>+
>+    e = q.expect('dbus-signal', signal='PresencesChanged')
>+    presences, = e.args
>+    type_, status, message = presences[hawk_handle]
>+    assertEquals(cs.PRESENCE_ERROR, type_)
>+    assertEquals('error', status)
>+    # FIXME: It might be less user-hostile to give some kind of readable
>+    # description of the error in future.
>+    assertEquals('internal-server-error', message)
>+
>+if __name__ == '__main__':
>+    exec_test(test)
>-- 
>1.7.7.3
>

Actions: View | Splinter Review

Attachments on bug 45491: 56438

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.