ServiceDiscoveryManager.discoverServices() throws Exception if one item throws

[Daniel Gultsch]

Hi,

I noticed that ServiceDiscoveryManager.discoverServices() throws an exception if one of the individual item discoveries throws an exception. That's unnecessary because there can be working services in that same list.

Compare the code in line 23ff https://github.com/iNPUTmice/ComplianceTester/blob/master/src/main/java/eu/siacs/compliance/tests/AbstractServiceTest.java#L23 (thats working) with the uncommented code below (how the api should be used)

[Christian Schudt]

Thanks, somewhat makes sense. The stacktrace of the exception or even the XMPP result would be interesting, but I guess it's some error like item-not-found.

[Daniel Gultsch]

I can provide you with a stack trace later but I think one of the potential reasons is the infamous echo service deplored on jabber.at for example that just reflects the stanza. So for your iq get you just get the same iq back (doesn't even modify it to result) so depending on strict babbler is with checking the result it either fails because of the wrong response type or because it can't find the item. In any case I think the discoverServices should not throw. Only when the original disco#items fails.

[Daniel Gultsch]

So here is the stacktrace. Turns out it's a s2s error. However what ever the error is it probably should not throw.

rocks.xmpp.core.stanza.StanzaException: service-unavailable  -  (type 'cancel': do not retry (the error cannot be remedied))
        No s2s connection found
    at rocks.xmpp.core.session.XmppSession.lambda$sendAndAwait$9(XmppSession.java:724)
    at rocks.xmpp.util.XmppUtils.lambda$notifyEventListeners$3(XmppUtils.java:192)
    at java.util.concurrent.CopyOnWriteArrayList.forEach(CopyOnWriteArrayList.java:890)
    at java.util.concurrent.CopyOnWriteArraySet.forEach(CopyOnWriteArraySet.java:404)
    at rocks.xmpp.util.XmppUtils.notifyEventListeners(XmppUtils.java:190)
    at rocks.xmpp.core.session.XmppSession.lambda$handleElement$16(XmppSession.java:1012)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

One more thing I noticed: discoverSerices() only checks for features on the disco#items and not the features of the server. Some services might announced on the server jid. (Whether this is actually a good idea is debatable.) Depending on how you configure your prosody for example it will announce the http file upload on the server entity and not a subdomain/item.

Not sure maybe you want to change the behavior here.

[Christian Schudt]

Discovering services should not fail immediately if one sub-query fails.

Fixes issue #77.

→ <<cset c50b0c4d7f18>>

[Christian Schudt]

Btw: discoverServices returns a list of disco#item items, which also have a node and a name, besides a jid attribute. Theoretically we could merge discoverServices() and discoverFeatures() into one method, which returns a list of JIDs, but I don't see a real use case for this, because all XEPs I've read use the approach, which is implemented (even XEP-0363).

[Christian Schudt]

Discovering services should not fail immediately if one sub-query fails.

Fixes issue #77.

→ <<cset 1ee760e8bb75>>

[Christian Schudt]

Fixed with version 0.7.1