Bug 35086 - wocky_xmpp_error_extract() should fill in @specialized_node for any specialized error.
Summary: wocky_xmpp_error_extract() should fill in @specialized_node for any specializ...
Status: RESOLVED FIXED
Alias: None
Product: Wocky
Classification: Unclassified
Component: General (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-03-07 08:02 UTC by Stef Walter
Modified: 2011-03-10 08:32 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Stef Walter 2011-03-07 08:02:10 UTC
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.
Comment 1 Stef Walter 2011-03-07 08:09:12 UTC
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
Comment 2 Will Thompson 2011-03-10 02:42:28 UTC
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. :))
Comment 3 Stef Walter 2011-03-10 04:34:57 UTC
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.
Comment 4 Will Thompson 2011-03-10 04:49:13 UTC
(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.
Comment 5 Stef Walter 2011-03-10 06:57:36 UTC
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.
Comment 6 Will Thompson 2011-03-10 08:32:10 UTC
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.