From ec7b2fe3e65cb457b07ad7b53d8818876af36b7b Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 1 Feb 2012 11:16:51 +0000 Subject: [PATCH] Presence: show better error status messages. When you get , any 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 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 #include #include +#include #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 in this presence, it's our own echoed back at + * us. So we don't want to use that. Instead, we use the 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