"unknown" connectivity status problematic

[Kyle Nitzsche]

metadata provides a scope a network connectivity status enum with three possible states:
* connected
* disconnected
* unknown

Sometimes unknown is the current state. This is quite problematic because in general a scopes that queries the network (that is, most scopes) need to know whether there is network or is not network and depending on this either make the network queries or display an error message.

A status of unknown connectivity leaves the scope with no valid option:
* the scope could assume the network is down and provide the user an error message. But often the network is not actually down, so this interrupts the user's flow unnecessarily
* the scope could assume the network is up, but this is dangerous if it is not up

[Marcus Tomlinson (marcustomlinson)]

I think the bug here is that you are seeing an Unknown status at all. That shouldn't happen. The shell sets the connectivity status before it dispatches queries, so you should only get Connected or Disconnected. Unknown is really more of an uninitialised state.

Could you please point me to the source code of a particular scope effected by this Unknown connectivity status issue.

[Michi Henning (michihenning)]

I strongly suspect that this may be related to this bug:

https://bugs.launchpad.net/ubuntu/+source/qtbase-opensource-src/+bug/1470700

The shell would be victimised by this just as much as the thumbnailer.

[Marcus Tomlinson (marcustomlinson)]

Michi, have a look at http://bazaar.launchpad.net/~unity-team/unity-scopes-shell/trunk/view/head:/src/Unity/scope.cpp#L745

The scope should actually never receive scopes::SearchMetadata::Unknown.

[Michi Henning (michihenning)]

Yes. But, underneath it, I suspect that QNetworkAccessManager isOnline() always returns false. (At least, that would be consistent with the behavior we saw in the thumbnailer.) Marcus, on line 173, Status::Unknown is mentioned.

Kyle, it's not clear from your original description whether you are *actually* seeing "Unknown", or are you just objecting to having that status in the enum? If the former, a test case would be really useful.

[Kyle Nitzsche (knitzsche)]

FYI I was on holiday last week and will add info here soon.

[Kyle Nitzsche (knitzsche)]

Michi,

1) Yes I object to even the possibility of ever receiving an Unknown so I proposed it be removed from the API
2) I have seen Unknowns in the past. However now I cannot reproduce.

[Michi Henning (michihenning)]

The tri-state accessibility comes straight from QNetworkAccessManager: http://doc.qt.io/qt-5/qnetworkaccessmanager.html#NetworkAccessibility-enum

There is a discussion thread here: http://www.qtcentre.org/threads/37514-use-of-QNetworkAccessManager-networkAccessible

I agree that the tri-state enum is useless. But the shell can't magically fix it, I suspect. The best option is probably to just assume that "Unknown" means the the same thing as "Not accessible". If bad things happen that way, so be it: inside the scope is the wrong place to fix this. It needs to be fixed in QNetworkAccessManager. (Failing that, maybe in the shell, but that still smells bad.)

[Michi Henning (michihenning)]

The poor doc for QNetworkAccessManager isn't helping :-(

I get the impression that the unknown state is caused by lazy initialisation: as long as no request has been sent yet, the state is unknown and, after the first request, changes to accessible or inaccessible as appropriate, until the underlying session changes.

If this is correct, the run time or the shell should probably do a dummy network access if the state is unknown and then send the actual state to the scope.

To me, this look like a poor design in QNetworkAccessManager. The idea of asking whether the network is accessible before sending a request is not that great because, if nothing else, it suffers from a race. Instead, QNetworkAccessManager should probably cache the state internally and, if a request is made while the network is disconnected, fail that request immediately. It really is a bit like access() and open(): I don't first call access() to try and work out whether, when I call open() it'll work; I just call open() directly and check whether it works.

[Timo Jyrinki (timo-jyrinki)]

It's clear the current Qt 5.4.1 has bugs related to this, but it's not clear how to fix those without causing regressions. It should be useful to experiment with the PPA https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/landing-032/ and try how things work with that. After upgrading with 'citrain device-upgrade 32 0000', double-check that especially 'dpkg -s libqt5network5' shows the version number '5.4.1+dfsg-2ubuntu11~vivid1~test1'.

That PPA now has commits that have been all merged in upstream Qt 5.5 branch, but the problem is that if you check up Weather app it reports connection not available. I didn't find another app that would have the same behaviour, and it might be just Weather app doing something weird regarding its network usage. But the silo can't be landed to vivid as long as there are regressions.

If 032 would resolve also this bug in addition to the 1470700, without seeing regressions elsewhere, then we really should find out what's wrong with the Weather app.

[Michi Henning (michihenning)]

Who looks after the Weather app? Whoever it is, they should be involved. Does anyone know?

[Michi Henning (michihenning)]

> * the scope could assume the network is up, but this is dangerous if it is not up

@Kyle: Why is it dangerous if it is not up?

[Kyle Nitzsche (knitzsche)]

@Michi, "dangerous" in the sense that the user experience is broken, as in, no "no network" result is posted, but also no actual results are posted, so the user sees something that looks broken.

[Tony Espy (awe)]

I agree with Kyle that "unknown" state is pretty useless, that said, as it's defined by QNetworkAccessManager, we can't really get rid of it without proposing a change to QNetworkAccessManager API. Thus I agree with Michi that for now scopes and/or the scopes framework should treat "unknown" the same as "disconnected".

[Michi Henning (michihenning)]

@Kyle: If no network is available, the network indicator should show it, I would think?

@Tony: I get the impression that treating "unknown" as "inaccessible" is not the right thing to do. From what I can glean, it appears that QNAM starts out in the "unknown" state and, once the first request has been made, settles into "accessible" or "inaccessible" accordingly. If that is the way it actually works, treating "unknown" as "inaccessible" would be exactly the *wrong* thing to do.

[Kyle Nitzsche (knitzsche)]

@Michi, yes the network indicator would I presume show a no network state. But the scope GUI itself would be in a state that could confuse a user and lead to invalid bugs, like this one: https://bugs.launchpad.net/today-scope/+bug/1507466

[Tony Espy (awe)]

@Michi

I guess I look at this from NetworkManager's point of view. Your either "connected" or you're not... The network indicator reflects this, it shows the mobile data technology if connected to the mobile operator. If WiFi is enabled, it works like the desktop. You can easily see the state of both WiFi and the mobile data connection by running the command 'nmcli d'.

Yea, the QNetworkAccessManager documentation isn't super clear. It also looks like there's a lot of control provided over network configuration, which may not actually work correctly on our platform.

With that said, it looks to me like "unknown" is the default accessible state, until the first request is created. From the thread you posted on qtcentre.org in comment #7, it appears that the accessible state can be primed:

"QNetworkAccessManager::networkAccessible is not explicitly set when the QNetworkAccessManager is created. It is only set after the network session is initialized. The session is initialized automatically when you make a network request or you can initialize it before hand with QNetworkAccessManager::setConfiguration() or the QNetworkConfigurationManager::NetworkSessionRequired flag is set."

It also includes sample code to do this:

    QNetworkAccessManager* mNetworkAccessManager = new QNetworkAccessManager();
    QNetworkConfigurationManager manager;
    mNetworkAccessManager->setConfiguration(manager.defaultConfiguration());

[Michi Henning (michihenning)]

> From the thread you posted on qtcentre.org in comment #7

I don't remember ever posting there. Do you have a link?

So, should the shell be doing what's suggested in the code example in order to give a defined status to the scope?

[Marcus Tomlinson (marcustomlinson)]

The discussion has derailed somewhat from the original issue, please continue this elsewhere.

Coming back to this particular bug, we only provide the statuses Connected and Disconnected to the scope from the shell. The Unknown status is an uninitialised state and should never reach the scope. Seeing that Kyle can no longer reproduce this, I'm setting the bug status to Incomplete.

[Timo Jyrinki (timo-jyrinki)]

Forgot to mention, but if you are interested in testing the silo 032 to test behaviour changes (I'm happy to hear about experiences), you need to add QT_EXCLUDE_GENERIC_BEARER=1 to /etc/environment.

[Kyle Nitzsche (knitzsche)]

A problem is that the QueryMetadata.h file [1] and the public docs [2] states that Unknown is one of the enum values. As such a scope dev needs to handle the case.

Perhaps those texts should be modified to indicate "Unknown" will never be the value, therefore sparing the dev the need to figure out how to handle it.

[1]
    \brief Indicates the internet connectivity status.

    The `Unknown` enumerator indicates that set_internet_connectivity() has not yet been called,
    hence the connectivity status is currently unknown.

[2]
https://developer.ubuntu.com/api/scopes/cpp/sdk-15.04.1/unity.scopes.QueryMetadata/

[Tony Espy (awe)]

@Michi

Hmmm, I expected lp to auto-link "comment #7" to the comment in this bug, but that apparently didn't work:

https://bugs.launchpad.net/ubuntu/+source/unity-scopes-api/+bug/1502282/comments/7

*If* it's possible for the shell to return "unknown", then yes I think the shell should be initializing the session per the example. That said, as I didn't write the code, and have zero direct experience with QNetworkAccessManager. I was just pointing out that this was suggested as a way to prevent "unknown" from ever being returned.

Anyways, it seems like there's not really anything more that I can contribute from a networking stack developer's perspective, so I'll abstain from further comments for now...

[Kyle Nitzsche (knitzsche)]

Problem: metadata.InternetConnectivity() is always returning 0 (for Unknown), where: metadata is the pointer passed into Activate().

http://bazaar.launchpad.net/~unity-team/go-unityscopes/v2/view/head:/metadata.go#L47

Don't know if this is a golang binding, c++ scope framework, unity8, network stack, or local coding issue.

But the local code is simple:

func (tt *dbScope) Activate(result *scopes.Result, metadata *scopes.ActionMetadata) (*scopes.ActivationResponse, error) {

         connStat := metadata.InternetConnectivity()

        log.Printf("== dropbox photos. Activate. Conn Status: %d\n", connStat)

That ^ prints the following, even though network is up and the indicator shows it as up:

2016/01/26 20:44:54 == dropbox photos. Activate. Conn Status: 0

[Michi Henning (michihenning)]

Kyle, do you see it returning unknown only before the first request, or always (even after a successful request)? I'd try treating "unknown" as "available" and see how that goes. There might still some weirdness in NetworkAccessManager.

[Kyle Nitzsche (knitzsche)]

I see different network connectivity state reported from Search() and from these: Activate(), Preview() and PerformAction().

I've written a simple golang scope [1] that shows the differences.

The scope has one result with SetInterceptAction(), so it goes to Activate, then it goes to Preview() where there is one button that runs PerformAction().

In each method I show the connection state as returned from the metadata passed into search AND that passed into the current method.

Here is the (commented) scope-registry.log: (value of 0 is unknown, 1 is connected, 2 is disconnected)

#WITH NETWORK

2016/01/28 22:43:37 ========= METATDATA TEST Search() ================================
2016/01/28 22:43:37 === Search(): Search's metadata.InternetConnectivity(): 1
# HERE I TAP THE RESULT WHICH RUNS Activate()
2016/01/28 22:43:42 === Activate(): Search's metadata.InternetConnectivity(): 1
2016/01/28 22:43:42 === Activate(): local's metadata.InternetConnectivity(): 0
2016/01/28 22:43:42 === Preview(): Search's metadata.InternetConnectivity(): 1
2016/01/28 22:43:42 === Preview(): local's metadata.InternetConnectivity(): 0
#HERE I TAP THE BUTTON WICH RUNS PerformAction()
2016/01/28 22:43:44 === PerformAction(): Search's metadata.InternetConnectivity(): 1
2016/01/28 22:43:44 === PerformAction(): local's metadata.InternetConnectivity(): 0

#WITH NO NETWORK (FLIGHT MODE ON) & REFRESHED SCOPE:

2016/01/28 22:44:36 ========= METATDATA TEST Search() ================================
2016/01/28 22:44:36 === Search(): Search's metadata.InternetConnectivity(): 2
2016/01/28 22:44:40 === Activate(): Search's metadata.InternetConnectivity(): 2
2016/01/28 22:44:40 === Activate(): local's metadata.InternetConnectivity(): 0
2016/01/28 22:44:40 === Preview(): Search's metadata.InternetConnectivity(): 2
2016/01/28 22:44:40 === Preview(): local's metadata.InternetConnectivity(): 0
2016/01/28 22:44:42 === PerformAction(): Search's metadata.InternetConnectivity(): 2
2016/01/28 22:44:42 === PerformAction(): local's metadata.InternetConnectivity(): 0

[1]
lp:~knitzsche/+junk/metadatatest

scope attached

[Kyle Nitzsche (knitzsche)]

@marcus: So yes, I would rather never see Unknown. But in any case I'd like the state to be consistent when returned by the different metadata objects. Is this still Incomplete?

[Michi Henning (michihenning)]

Hmmm... This looks like the ActionMetadata that is passed in has an uninitialised connectivity status. I'm not sure who's to blame here. It might be the Go binding, or it might be shell.

[Kyle Nitzsche (knitzsche)]

It seems there are two issues here:

1) As noted, apparently ActionMetadata passed into PerformAction and Activate methods is always Unknown

2) Network connectivity status should not be passed into the scope as an enum because it can become stale and therefore can prevent a scope from making good decisions based on *current* network status. Consider the case where a user displays a Preview, puts the phone down, and comes back an hour later, then clicks a Preview button. The code might want to do different things if there is network and if there is not.

[Michi Henning (michihenning)]

You are always free to ignore the network status. It was moved in mainly as a convenience feature, to allow a scope to immediately notice if, for example, the device is in flight mode. Given the problems with NetworkAccessManager (still not completely resolved), I would probably interpret "unknown" to mean "available" and go ahead, and only pay attention if the status indicates "network down".

The issue with ActionMetadata and PerformAction not receiving the network status needs looking at.

[Michi Henning (michihenning)]

Here is the code in QueryMetadata that set this:

it = var.find("internet_connectivity");
    if (it != var.end())
    {
        internet_connectivity_ = it->second.get_bool() ? QueryMetadata::Connected : QueryMetadata::Disconnected;
    }
    else
    {
        internet_connectivity_ = QueryMetadata::Unknown;
    }

So, the only way for this not to be set is if the status isn't passed in by the caller.

ActionMetadata and PerformAction derive from QueryMetadata, so they get this setting for free. At least going by this brief look, I'd say that the shell doesn't set the status for activation and preview.

[Paweł Stołowski (stolowski)]

Michi, your guess was correct, the shell doesn't set connectivity status in ActionMetadata. Attached branch fixes that.

[Launchpad Janitor (janitor)]

This bug was fixed in the package unity-scopes-shell - 0.5.7+16.10.20160525-0ubuntu1

---------------
unity-scopes-shell (0.5.7+16.10.20160525-0ubuntu1) yakkety; urgency=medium

  [ Gary.Wzl ]
  * Stop typing timer when reset navigation tag(tapping cancel). Also
    make sure relevant signal(searchQueryChanged) is triggered to keep
    query string updated as query string is empty after tapping cancel
    (LP: #1576311)

  [ Marcus Tomlinson ]
  * Only create an empty settings file when attempting to write to one
    (LP: #1583055)

  [ Pawel Stolowski ]
  * Don't depend on qt5-default.
  * Reject preview widgets with duplicated IDs. (LP: #1581495)
  * Set internet connectivity status in ActionMetadata (previews and
    preview actions). (LP: #1502282)
  * Workaround dependency issue on yakkety.

 -- Pawel Stolowski <email address hidden> Wed, 25 May 2016 08:16:27 +0000

[Paweł Stołowski (stolowski)]

Marking invalid in scopes API since the problem was in the shell plugin and got fixed.