The "Federated Learning" shield study is not removed after its expiration date

[Softvision-RemusDranca]

[Affected versions]:

• Firefox 62.0b6

[Affected Platforms]:

• All Windows
• All Mac
• All Linux

[Prerequisites]:

• Have a new Firefox profile with the federated-learning@shield.mozilla.org-1.0 build installed.
• Have 'extensions.legacy.enabled' pref set to 'true'.

[Steps to reproduce]:

1. Set the system date to two weeks after the current date.
2. Open the browser with the profile from prerequisites and navigate to "about:addons">Extensions section.
3. Observe the addons section.

[Expected result]:

• The "Federated Learning" shield study is uninstalled and no longer displayed in the add-ons section.

[Actual result]:

• The "Federated Learning" shield study remains installed and displayed in the add-ons section.

[Notes]:

• This issue is reproducible in all branches ("control", "treatment" and "control-no-decay").
• Attached is a screen recording of the issue:

[florian]

@gregglind Is this a bug in the shield-studies-addon-utils repo or do I need to add additional code?

I tried using this code:

browser.study.onEndStudy.addListener(() => {
  optimizer.reset()
  browser.management.uninstallSelf()
})

Or, alternatively, just logging something instead of calling uninstall.
The callback is not triggered if the time is over.
I tested changing the system time as well as changing the shield.federated-learning_shield_mozilla_org.firstRunTimestamp pref to a very small value.

"Ending a study" in the FAQ makes it sound like the callback should be triggered here.

[gregglind]

Agreed it should be. Will debug tomorrow.

…

On Tue, Jul 10, 2018 at 3:55 PM Florian Hartmann ***@***.***> wrote: @gregglind <https://github.com/gregglind> Is this a bug in the shield-studies-addon-utils <https://github.com/mozilla/shield-studies-addon-utils/> repo or do I need to add additional code? I tried using this code: browser.study.onEndStudy.addListener(() => { optimizer.reset() browser.management.uninstallSelf() }) Or, alternatively, just logging something instead of calling uninstall. The callback is not triggered if the time is over. I tested changing the system time as well as changing the shield.federated-learning_shield_mozilla_org.firstRunTimestamp pref to a very small value. "Ending a study" in the FAQ <https://github.com/mozilla/shield-studies-addon-utils#faq> makes it sound like the callback should be triggered here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAj6wEHl_H_FirBLfgwBcEmX8B0Cvyks5uFRSjgaJpZM4VJv_u> .

[gregglind]

I am working on this. Automated tests work correctly. Trying to figure out what if anything is different about your procedure and usage.

[gregglind]

Note: I consider this NOT A BLOCKER, because we can hard kill studies.

Hard kill: Normandy forces an addon uninstall.

These are the risks:

• survey will not be launched
• addon code in the WE layer will not be run.

mozilla/shield-studies-addon-utils#244
mozilla/shield-studies-addon-utils#246

[motin]

This bug was fixed in utils 5.0.2. I don't see what version is used in this add-on. Could you add utils as a dependency to package.json? See https://github.com/mozilla/shield-studies-addon-utils#installing-the-shield-studies-addon-utils-in-your-add-on - thanks :)

[gregglind]

(You have 5.0.1, trying to patch for you)

[florian]

@motin, @gregglind: Thanks, I wasn't aware of that!

The upgrade and #11 don't seem to fix this for me however.
I tried triggering the expiry by changing the system date / changing the respective timestamp pref.
If I then restart the browser, the pref is cleared to an empty value and studyInfo.delayInMinutes is null, so the alarm is not created.

[motin]

@florian Expiration is a bit tricky. I recommend comparing your implementation with the one in the template: https://github.com/mozilla/shield-studies-addon-template

The template also has functional tests that verifies that the expiration works as expected. You might want to lift in similar functional tests to your repo (like in mozilla/shield-cloudstorage#48)

There are also some new docs coming regarding expiration, available here: https://github.com/mozilla/shield-studies-addon-utils/pull/248/files#diff-c320e75ed07805a5316c10c23ba7d297

If all fails, try retrofitting your study-logic into a fork of the template so that you have a better-tested base for the study add-on.