Race condition waiting for welcome mods to be activated on first run

[twschiller]

Describe the bug

• Hypothesis that some starter mods are being activated after the welcome page is displayed and disrupting user first run experience: https://pixiebrix.slack.com/archives/C02RAMRDF0R/p1723762868646669?thread_ts=1723762436.705289&cid=C02RAMRDF0R
• https://www.notion.so/pixiebrix/Welcome-Mod-Activation-Reliability-2024-08-16-4664ba7056c744398ebfce68f9bdfb60?pvs=4

• pixiebrix-extension/src/background/starterMods.ts

  Line 312 in fad8a82

  export const debouncedActivateStarterMods = debounce(

Actual behavior

• Some mods are activated after the welcome page is displayed to the user (from product telemetry)

Discussion

• @mnholtz let me know if I need to move this issue to pixiebrix-app repository

[mnholtz]

let me know if I need to move this issue to pixiebrix-app repository

I'm pretty sure this is the right repo

[mnholtz]

Documenting my investigation so far because I have been unable to replicate my hypothesis of what is going on.

My hypothesis is that because we've debounced _activateStarterMods in starterMods.ts, we are no longer waiting for welcome mods to actually be activated before proceeding with onboarding flow logic. This would hypothetically result in this logic getting prematurely skipped over, which would result in the user not being taken to the welcome page after onboarding (although they would get welcome mods).

For example:

import { debounce } from 'lodash';

const DEBOUNCE_MS = 10_000;
const MAX_MS = 60_000;

const asyncHello = async () => {
  await setTimeout(() => {
    console.log('hello');
  }, 2000);
};

const debouncedHello = debounce(asyncHello, DEBOUNCE_MS, {
  leading: true,
  trailing: false,
  maxWait: MAX_MS,
});

debouncedHello();
debouncedHello();
debouncedHello();
console.log('hello first');

// Output
// hello first
// hello

However, due to the fact that I can't get a consistent console to be open during onboarding, I'm struggling to confirm that hypothesis.

Some thoughts here:

1. While my above playground example seems to yield that result, it doesn't seem to actually behave that way in practice? e.g. I added a sleep to _activateStarterMods and ran the debounced external messenger method as-is, and we do seem to wait for that obviously long time (see here: https://github.com/pixiebrix/pixiebrix-extension/compare/bugfix/9014_welcome_mod_race?expand=1)
2. We apparently originally started debouncing this method due to the onUpdate browser extension event. Add onUpdate event listener for installing starter blueprints on Playground urls #4249 I'm not sure how relevant this is anymore, as we did this two years ago. We might consider just removing the debounce.
3. The return value from _activateStarterMods is a boolean based on whether or not mods have been activated (and does not give any more insight into why they might have not been activated). We might consider redesigning the return value to be more helpful.
4. We might consider an alternative flow of always taking users to the welcome page regardless of welcome mod activation success. This is just something to consider, possibly for future work

Additional resources

• See this branch for some of my experiments: https://github.com/pixiebrix/pixiebrix-extension/compare/bugfix/9014_welcome_mod_race?expand=1

[grahamlangford]

@twschiller any thoughts on this suggestion?

2. We apparently originally started debouncing this method due to the onUpdate browser extension event. Add onUpdate event listener for installing starter blueprints on Playground urls #4249 I'm not sure how relevant this is anymore, as we did this two years ago. We might consider just removing the debounce.

I also want to look into this suggestion:

3. The return value from _activateStarterMods is a boolean based on whether or not mods have been activated (and does not give any more insight into why they might have not been activated). We might consider redesigning the return value to be more helpful.

[twschiller]

We apparently originally started debouncing this method due to the onUpdate browser extension event. #4249 I'm not sure how relevant this is anymore, as we did this two years ago. We might consider just removing the debounce.

I think the reason it's there is because: 1/ starter mods are activated on first run, 2/ starter mods are activated when the user visits the welcome page. So, on the initial install where the user gets redirected to the welcome page, they'd end up getting the welcome mods 2x

From the PR description, it seems it also helped avoid a scenario on SPAs (which might have been a problem with our initial playground) where the page would trigger multiple navigation events. That part is likely less/not relevant now for the current welcome page

I also want to look into this suggestion:

Main concern would be ensuring the app code is backward compatible

[grahamlangford]

@twschiller we had this discussion back in April: https://github.com/pixiebrix/pixiebrix-extension/pull/8290/files#r1572440502

I'm going to pull on this thread for a bit