1855545 - ShoppingUtils.init invokes NimbusFeatures.onUpdate in a loop

Status: VERIFIED FIXED

(Firefox :: Shopping, defect, P1)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

The ShoppingUtils.init code assumes that NimbusFeatures.shopping2023.onUpdate(callback) does not immediately invoke the callback when it is called, but it does. This leads to a "too much recursion" error on startup if enrolled in the beta test experiment, following these steps:

1. In about:config set nimbus.debug to true
2. open about:studies?optin_slug=review-checker-first-beta-119&optin_branch=treatment-a&optin_collection=nimbus-preview
3. restart the browser
4. look in the browser toolbox, observe a "Too much recursion" error calling the following functions in a loop:

InternalError: too much recursion
    _onFeatureUpdate resource://nimbus/lib/ExperimentStore.sys.mjs:389
    onUpdate resource://nimbus/ExperimentAPI.sys.mjs:472
    init resource:///modules/ShoppingUtils.sys.mjs:61
    onNimbusUpdate resource:///modules/ShoppingUtils.sys.mjs:43
    _onFeatureUpdate resource://nimbus/lib/ExperimentStore.sys.mjs:389
    onUpdate resource://nimbus/ExperimentAPI.sys.mjs:472
    init resource:///modules/ShoppingUtils.sys.mjs:61
    onNimbusUpdate resource:///modules/ShoppingUtils.sys.mjs:43
   ...

The problem here is that ShoppingUtils.init, inside the if (!this.registered) block, doesn't immediately set this.registered = true, but instead does:

   if (!this.registered) {
      lazy.NimbusFeatures.shopping2023.onUpdate(this.onNimbusUpdate);
      this._updateNimbusVariables();
      this.registered = true;
    }

It turns out that the onUpdate call immediately invokes this.onNimbusUpdate, which in turn calls ShoppingUtils.init, causing an infinite loop.

We can fix this by moving the this.registered = true line to the top of the block.

Comment 1

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Bug 1855545: Avoid an infinite loop in ShoppingUtils.init r?mardak

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce22a7ec1ea0
Avoid an infinite loop in ShoppingUtils.init r=Mardak

Comment 3

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ce22a7ec1ea0

Comment 4

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 9355410 [details]
Bug 1855545: Avoid an infinite loop in ShoppingUtils.init r?mardak

Beta/Release Uplift Approval Request

• User impact if declined: The shopping experiment will not initialize correctly, which will break some experiment telemetry.

• Is this code covered by automated tests?: Yes

• Has the fix been verified in Nightly?: No

• Needs manual test from QE?: Yes

• If yes, steps to reproduce: Opt in to the Nimbus experiment, then check for console errors and verify the telemetry is working.

• in about:config, set nimbus.debug to true

• load about:studies?optin_slug=review-checker-first-beta-119&optin_branch=treatment-a&optin_collection=nimbus-preview

• restart the browser

• visit a supported product page

• check the value of the probes Glean.shoppingSettings.componentOptedOut, Glean.shoppingSettings.hasOnboarded, Glean.shoppingSettings.nimbusDisabledShopping have been set

• List of other uplifts needed: None

• Risk to taking this patch: Low

• Why is the change risky/not risky? (and alternatives if risky): The shopping experiment will be limited to a very small audience.

This change is extremely small and confined to the shopping experiment initialization code.

• String changes made/needed: No
• Is Android affected?: No

Comment 5

[Rares Doghi, Desktop QA]

This issue is Verified as fixed in our latest Nightly build. We are being enroled and we no longer see the recursion error.

The telemetry pings we see in glean are :
"shopping.settings.component_opted_out": false,
"shopping.settings.nimbus_disabled_shopping": false,
"shopping.settings.has_onboarded": true,

Comment 6

[Dianna Smith [:diannaS]]

Comment on attachment 9355410 [details]
Bug 1855545: Avoid an infinite loop in ShoppingUtils.init r?mardak

Approved for 119.0b4

Comment 7

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/6efd447cf80f

Comment 8

[Rares Doghi, Desktop QA]

Verified as fixed in 119.0b4.