Currently Wocky has a hodge-podge of error parsing functions copied from Gabble. Some of them produce GErrors, but don't get the error text correctly; some produce a whole pile of pointers to nodes, strings and whatnot; etc. Also, the error node production functions only work with core stanza errors; if you want to produce, say, Jingle errors, you're out of luck. My 'error-wrangling' branch adds the concept of specialized error domains, whereby you can register a GQuark (for the GError domain whose string happens to be the XMPP namespace) and a GEnum representing application-specific errors, along with which XMPP Core stanza errors they specialize. Then, it improves the error stanza production code to produce a full <error/> stanza from GErrors in those domains (or in the basic WOCKY_XMPP_ERROR domain, as supported before), and adds a function to break down a stanza error into the type of error, a GError for the core error, and a GError for the specialized error. (This split is needed so that you can get the core error if you don't understand the specialization.) Then I went through and deleted all the superseded functions. It was a nice feeling. == Outstanding points == Maybe we can't use the XML namespace as the GError quark. Consider the following case: • A Wocky-based application implements a new XEP, with errors in the foo:bar namespace. • Subsequently, Wocky includes that error domain itself, but enumerates the error codes in a different order. • Now one of those orders of errors has to win, breaking either Wocky itself, or the application using it. One way out of this trap would be to make wocky_xmpp_error_extract() take a list of specialized domains the calling code understands, and break up WockyXmppErrorDomain.domain into one quark for the GError domain, and another for the XML namespace. Unrelatedly, some of the naming isn't great. I'm not overly fond of WockyXmppErrorDomain; perhaps WockyErrorNS? WockyXmppError should probably be WockyStanzaError or something. Finally, wocky_stanza_error_to_node() and wocky_xmpp_error_extract() are essentially inverses but have different-shaped names; I'm not sure which side of this fence to fall on. Suggestions? tests/wocky-connector-test.c | 16 +- tests/wocky-xmpp-node-test.c | 182 --------- tests/wocky-xmpp-stanza-test.c | 363 ++++++++++++++++-- wocky/Makefile.am | 2 +- wocky/wocky-connector.c | 183 ++++------ wocky/wocky-muc.c | 53 +-- wocky/wocky-porter.c | 2 +- wocky/wocky-roster.c | 2 +- wocky/wocky-utils.c | 67 ++++ wocky/wocky-utils.h | 3 + wocky/wocky-xmpp-error.c | 814 +++++++++++++++++++++++----------------- wocky/wocky-xmpp-error.h | 85 +++-- wocky/wocky-xmpp-node.c | 20 +- wocky/wocky-xmpp-node.h | 3 + wocky/wocky-xmpp-stanza.c | 88 +++-- wocky/wocky-xmpp-stanza.h | 10 +- wocky/wocky.c | 3 + 17 files changed, 1121 insertions(+), 775 deletions(-)
(I have a corresponding Gabble branch that updates it for the changes this Wocky branch introduces.)
d915abf0858f13c000203685bbfce8368803dd60: The while loop doesn't actually iterate the list of children, causing an infinite loop if the error isn't the first child. Seems to be removed in a later commit 5c88062e9470be54720a5bca1a4a3276f028e578 First hunk has if (!quark), all others do if (quark == 0). 4da39e6569b55ab29610e3f27c2de3ade4db5acd Nitpick, i'm a sucker for user g_slist_next instead of ->next. But that's just me 86490fc3bcf9c87b0b7be9d6eaadadbc9cbf1c01 Nitpick, this assumes wocky_enum_from_nick doesn't change type_i iff it couldn't find the type. Might be more defensive to check the return and only then set it to TYPE_CANCEL
(In reply to comment #2) > d915abf0858f13c000203685bbfce8368803dd60: > > The while loop doesn't actually iterate the list of children, causing an > infinite loop if the error isn't the first child. Seems to be removed in a > later commit Good catch. But it is indeed removed later. I could edit the past, but it doesn't seem too necessary. > 5c88062e9470be54720a5bca1a4a3276f028e578 > > First hunk has if (!quark), all others do if (quark == 0). Yep; fixed in 533bbdc. > 4da39e6569b55ab29610e3f27c2de3ade4db5acd > > Nitpick, i'm a sucker for user g_slist_next instead of ->next. But that's just > me I don't think it's any clearer, and it's more typing. :) > 86490fc3bcf9c87b0b7be9d6eaadadbc9cbf1c01 > > Nitpick, this assumes wocky_enum_from_nick doesn't change type_i iff it > couldn't find the type. Might be more defensive to check the return and only > then set it to TYPE_CANCEL Instead, I documented this assumption in enum_from_nick()'s docstring: f6842a6 Gabble branch on its way...
Here's a Gabble branch with minimal changes to work with a new snapshot of Wocky: http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=shortlog;h=refs/heads/error-wrangling
Both the Wocky branch and the Gabble branch are merged, with Sjoerd's approval! Will be in Gabble 0.9.5.
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.