"unknown" connectivity status problematic

Bug #1502282 reported by Kyle Nitzsche
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
The Savilerow project
New
Undecided
Unassigned
unity-scopes-api (Ubuntu)
Invalid
Undecided
Unassigned
unity-scopes-shell (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

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

Related branches

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

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.

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

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.

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

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

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

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.

Revision history for this message
Michi Henning (michihenning) wrote :

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.)

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

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.

Revision history for this message
Michi Henning (michihenning) wrote :

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

Revision history for this message
Michi Henning (michihenning) wrote :

> * 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?

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

@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.

Revision history for this message
Tony Espy (awe) wrote :

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".

Revision history for this message
Michi Henning (michihenning) wrote :

@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.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

@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

Revision history for this message
Tony Espy (awe) wrote :

@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());

Revision history for this message
Michi Henning (michihenning) wrote :

> 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?

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

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.

Changed in unity-scopes-api (Ubuntu):
status: New → Incomplete
Changed in unity-scopes-shell (Ubuntu):
status: New → Incomplete
Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

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.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

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/

Revision history for this message
Tony Espy (awe) wrote :

@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...

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

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

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

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

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

@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?

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Changed in unity-scopes-api (Ubuntu):
status: Incomplete → Confirmed
Changed in unity-scopes-shell (Ubuntu):
status: Incomplete → Confirmed
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

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.

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Revision history for this message
Michi Henning (michihenning) wrote :

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.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in unity-scopes-shell (Ubuntu):
status: Confirmed → Fix Released
Revision history for this message
Paweł Stołowski (stolowski) wrote :

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

Changed in unity-scopes-api (Ubuntu):
status: Confirmed → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.