In order to use wocky_xmpp_error_extract() properly, we need to not only be able to parse error codes that have been registered. We should be filling in @specialized_node with a node that matches a registered error domain, regardless of whether the actual specific code matches or not.
Implemented in error-always-specialized-node branch in my wocky.git repo: http://git.collabora.co.uk/?p=user/stefw/wocky.git;a=commit;h=f5b06e8862884df68e6947be3747bb794e0a3a54
I'm not sure about this patch. Why would Wocky ever know about an error namespace, but not know about one of the errors in that namespace? Stanza errors have the form: <error> <defined-condition xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/> <text xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'>...</text> <!-- and optionally --> <any-element xmlns='any:namespace'> ... </any-element> </error> With your patch, I think @specialized_node would be set if and only if any:namespace had previously been registered with Wocky; but surely it would be more useful to *always* set @specialized_node to the (first) element not in xmlns='urn:ietf:params:xml:ns:xmpp-stanzas', if one exists? (Or perhaps I'm misreading the patch. Either way, adding a test case alongside the existing extract_errors() tests in tests/wocky-stanza-test.c would help clarify the intent, and verify that it works. :))
Can there (In reply to comment #2) > I'm not sure about this patch. Why would Wocky ever know about an error > namespace, but not know about one of the errors in that namespace? Because the namespace has an extensible or indeterminate set of errors? > Stanza errors have the form: > > <error> > <defined-condition xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/> > <text xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'>...</text> > > <!-- and optionally --> > <any-element xmlns='any:namespace'> ... </any-element> > </error> Can there only be one <any-element>? > With your patch, I think @specialized_node would be set if and only if > any:namespace had previously been registered with Wocky; but surely it would be > more useful to *always* set @specialized_node to the (first) element not in > xmlns='urn:ietf:params:xml:ns:xmpp-stanzas', if one exists? I can do this. But if more than one <any-element> type specialized node can be present, this raises the obvious question of whether @specialized should always match @specialized_node or not. > (Or perhaps I'm misreading the patch. Either way, adding a test case alongside > the existing extract_errors() tests in tests/wocky-stanza-test.c would help > clarify the intent, and verify that it works. :)) Okay, will do.
(In reply to comment #3) > Can there (In reply to comment #2) > > I'm not sure about this patch. Why would Wocky ever know about an error > > namespace, but not know about one of the errors in that namespace? > > Because the namespace has an extensible or indeterminate set of errors? But they don't. Take Jingle, for instance: it defines an XML namespace with a fixed set of errors. <http://xmpp.org/extensions/xep-0166.html#errors> If a new error is added to that set later, then the application has to be updated anyway to handle it, so Wocky's list of errors could be updated too. > > Stanza errors have the form: > > > > <error> > > <defined-condition xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/> > > <text xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'>...</text> > > > > <!-- and optionally --> > > <any-element xmlns='any:namespace'> ... </any-element> > > </error> > > Can there only be one <any-element>? Yes, by my reading of <http://xmpp.org/rfcs/rfc3920.html#stanzas-error> there may be up to three children of <error>: a condition in ...:xmpp-stanzas, <text/> in ...:xmpp-stanzas, and one other element in a different namespace. I think the code in Wocky is a bit too tolerant. > > With your patch, I think @specialized_node would be set if and only if > > any:namespace had previously been registered with Wocky; but surely it would be > > more useful to *always* set @specialized_node to the (first) element not in > > xmlns='urn:ietf:params:xml:ns:xmpp-stanzas', if one exists? > > I can do this. But if more than one <any-element> type specialized node can be > present, this raises the obvious question of whether @specialized should always > match @specialized_node or not. Yeah, I think it's reasonable to say that if @specialized is set, it should match @specialized_node. > > (Or perhaps I'm misreading the patch. Either way, adding a test case alongside > > the existing extract_errors() tests in tests/wocky-stanza-test.c would help > > clarify the intent, and verify that it works. :)) > > Okay, will do.
Thanks. Made changes to reflect your suggestions. http://git.collabora.co.uk/?p=user/stefw/wocky.git;a=commit;h=7f29376886b7f87d75077f7a0e940c755b1415aa This depends on the test-one-thing branch, found in the same repo.
Merged; thanks!
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.