fix: [M3-6586] - Improve error handing for loadScript and Adobe Analytics

[mjac0bs]

Description 📝

This is an attempt to improve the loadScript function and error handling for its use with Adobe Analytics.

Major Changes 🔄

• Removes the error logged when a custom event fires unsuccessfully
  • This will get rid of the annoying Sentry errors that are currently being triggered when any user blocking the analytics scripts performs an action that would fire a custom event to track, if analytics were enabled.
• Modifies loadScript function to return a promise.
• Attempts to verify that all three scripts necessary for Adobe (the launch script and its two extensions) are present in the DOM if the script claims it has loaded successfully with a resolved promise.
  • "Attempts" because I haven't found a way to successfully stop all 3 scripts from loading, even with assets.adobedtm.com/extensions/*requests blocked, to come closer to mimicking what we saw with the extensions not fully loading (200 response but in pending state and not present in the DOM) a week ago.

How to test 🧪

1. How to setup test environment?
   • Check this PR out locally and have the REACT_APP_ADOBE_ANALYTICS_URL environment variable defined in your .env.
   • Type _satellite.setDebug(true) in the developer console and refresh the page. You should see some debug logs to indicate that Adobe Analytics is running.
2. How to reproduce the issue (if applicable)?
   • To verify an error on custom events is logged:
     • Check out develop locally.
     • Use dev tools to block requests to assets.adobedtm.com.
     • Trigger a custom event (e.g. create a linode or volume).
     • Observe that an exception is logged to the browser console; in prod, this would be logged to Sentry.
3. How to verify changes?
   • To verify an error on custom events is not logged:
     • Use dev tools to block requests to assets.adobedtm.com.
     • Trigger a custom event (e.g. create a linode or volume).
     • Observe that an exception is no longer logged.
   • Check that there have been no regressions in the places the app calls useScript, such as the Google Pay button and chip on the Billing page. (Try blocking network requests and testing.) I had to slightly modify this hook when I changed the loadScript function.
   • With requests to assets.adobedtm.com blocked, confirm that the app works as expected and no new console errors/warnings are logged.

[cypress]

1 failed and 1 flaky tests on run #3792 ↗︎

1

165

3

0

1

Details:

Address feedback
Project: Cloud Manager E2E	Commit: 9676ae7d46
Status: Failed	Duration: 16:57 💡
Started: May 24, 2023 8:57 PM	Ended: May 24, 2023 9:14 PM

  cypress/e2e/volumes/resize-volume.spec.ts • 1 failed test

View Output Video

Test		Artifacts
volume resize flow > resizes a volume		Output Screenshots Video

  cypress/e2e/firewalls/update-firewall.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
update firewall > updates a firewall's linodes and rules		Output Screenshots Video

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[mjac0bs]

I logged an exception at this point because this would indicate that the analytics scripts were not blocked by the user and did (supposedly) load on the page, but still aren't set up as expected, so something is really wrong. I haven't been able to figure out a way to trigger any of these conditions, though, and am open to feedback on whether we'd want to log this to Sentry at all.

[jdamore-linode]

I know this is all hypothetical and challenging to actually test, but do we know if this actually makes any impact? If the launch script has already loaded and set up the _satellite object, do we know that removing the <script /> element actually does anything to stop its execution going forward?

[mjac0bs]

If the scripts don't exist, the _satellite object doesn't either -- I think it's loaded in the AppMeasurement.tsx extension. The thought behind removing all the scripts that do exist at this point is mostly that if the launch script were to load, but either/both of the extensions, for some reason, didn't, then we'd just remove the launch script all together since it won't work without the extensions (and maybe prevent a crash from continuing to try to load them?).

But this is hypothetical, you're right, and if there are reasons we'd want to consider not doing this, I'm happy to rethink this.

[dwiley-akamai]

What should the exception look like? This is what I see, both on this branch and on develop:

[dwiley-akamai]

Is there a reasonable likelihood that the number of Adobe script tags could change in the future? If so, that could cause this logic to break data collection.

[mjac0bs]

Yeah, that's a good question -- asked internally about this and am waiting on a response.

Thinking of a scenario where a decision is made to modify the launch script and add another extension... if that was communicated to our team in advance, we'd need to remember to update this number (which I should have defined in constants.ts). If, for some reason, anything changed with Adobe unexpectedly and we had a different number of scripts, it would stop data collection. In either case, I wouldn't expect a frequent change to the number of scripts. We could opt to not remove any scripts, just log the error, and hopefully rely on monitoring tests in the future to let us know if scripts are causing any issues with loading the app.

[mjac0bs]

I took out the logic to completely remove the scripts for the time being considering this point and Joe's question. Keeping the exception as is to log the issue, though, since we're not anticipating the number of tags changing frequently (if at all).

[mjac0bs]

@dwiley-akamai My bad on not specifying what the exception was more clearly in the repro steps (just fixed). The exception you should see on develop is this one when blocking requests. And on the PR’s branch, you shouldn’t see the above exception or this exception just because you’ve blocked analytics from tracking. You will still see the Adobe debug log error in both cases - we don't control that one.

[hkhalil-akamai]

Confirming that the exception is no longer thrown.

[cpathipa]

Changes LGTM! I don't see any regressions in Google pay and billing page.
Observation: Was not able to produce the exception either in develop as well as PR branch.