feat(umami-plugin): inject mock on umami script download error

[JulesFiliot]

Fixes the error raised here acid-info/codex.storage#59 (comment)

After taking more time to review it, the problem is actually that when the umami script is blocked from being downloaded by the client, we get an umami undefined error afterwards when trying to invoque the track function from it.

So the problem is not that we allow the script to be downloaded on other domains than the one specified in the dataDomains property (c.f. acid-info/codex.storage#59 (comment)). If the script is downloaded on any other domains than the one.s specified, nothing will be tracked and no error will ever be triggered as the umami object will be correctly defined. So all good.

So my approach was to inject the mock umami object, as suggested by @emizzle, only when downloading the script gets blocked by the client thus triggering an error.
The mock contains two functions, track and identify; the code here from Umami's repo shows that track should be enough, but the Umami docs here mention an identify function, so to play it safe I added both.

Next step after merge:

• Test on develop version of codex.storage (I tested locally before ofc)
• Bump every docusaurus website's @acid-info/docusaurus-umami package's version.

[emizzle]

Thanks for doing this Jules.

Regarding testing, I'd recommend figuring out a way to test this prior to merging. The easiest way would be to remove the window.umami object before the test runs.

One other important note, we removed the button from the codex website that has this umami tracking script, however it is easily testable (manually) in a brave browser.

[emizzle]

This seems like a very strange way to add a head tag honestly. If I understand this correctly, it adds a head tag that then adds another head tag that had the onerror attribute set. I'm assuming you tried adding the onerror attribute first, eg

const attributes = {
    defer: true,
    src: scriptSrc,
    'data-website-id': websiteId,
    onerror: function onError() {
        if(!window.umami){
            console.warn('[docusaurus-plugin-umami] Failed to load Umami script - using mock tracker instead.');
            window.umami = {
              track: function(){},
              identify: function(){}
            }
          }
    }
}

Another option, according to the plugin docs, is to return a scripts object, eg

return {
  scripts: [
  {
        src: scriptSrc,
        defer: true,
        'data-website-id': websiteId,
        onerror: function onError() {
        if(!window.umami){
            console.warn('[docusaurus-plugin-umami] Failed to load Umami script - using mock tracker instead.');
            window.umami = {
              track: function(){},
              identify: function(){}
            }
          }
    }
    },
  ],
}

[JulesFiliot]

Yes the implementation is a bit "complex" because there was no other way to be able to inject the mock when the client throws an error when downloading the tracking script from umami.

• Approach 1: It won’t work because functions in attributes are serialized as strings by docusaurus, so the onerror handler never actually runs. It will give something like "[object Function]".

• Approach 2: It won’t work either because returning a scripts array isn’t part of the docusaurus plugin API, plugins must inject scripts via injectHtmlTags().
  The docs you linked are the one for docusaurus.config.js to, among other things, import and configure external plugins. For the docs related to creating a plugin, you can refer to this doc for the available methods in a plugin and this one for the details regarding injectHtmlTags()

[JulesFiliot]

@emizzle thanks for your review 🙏

As I've said here, there was really no other way to inject the mock upon triggering an error via the head tag.

However when scrolling the docs I realized that we could use client modules instead to have a cleaner implementation than the "complex" one I had to previously do. So I've refactored the code into something more digest and simple using client modules. Please tell me what you think - no need for onerror anymore, the mock is injected before everything so absolutely no risk of triggering an error and if the umami script successfully loads it will overwrite the mock.

Regarding testing, I'd recommend figuring out a way to test this prior to merging. The easiest way would be to remove the window.umami object before the test runs.

As I've said on the description of the PR, I already found a way to test locally. Since I use Brave browser and it is the browser triggering the error it's quite straightforward:

• Start a docusaurus project locally using the umami plugin
• In Brave browser turn Brave shield off
• Navigate to http://localhost:3000/
• In the console type umami & enter to check that the object correctly exposes the track and identify methods from https://umami.bi.status.im/script.js
• Then turn Brave shield on & refresh the page
• Check the umami object again and it should expose the two same methods but from umami-mock.js this time.

[emizzle]

THanks Jules, this is much cleaner.

As I've said on the description of the PR, I already found a way to test locally. Since I use Brave browser and it is the browser triggering the error it's quite straightforward:

Apologies for the misunderstanding. What I meant was that it would be prudent to figure out an automated way of testing this, eg with a unit test. A unit test would allow this to be confirmed in other environments (eg CI) and it prevents future regressions.