🐛 [RUM-8429] Report an error when lazy loading the recorder module fails

[sethfowler-datadog]

Motivation

Lazy loading the recorder module can fail, either due to a customer-side issue (like a CSP misconfiguration) or due to an issue on our end (like failing to upload the chunk to the CDN). We should report an error when this happens and add telemetry.

The current situation is a bummer since the failure is totally silent; no error is reported anywhere. I spent quite a bit of time over the past couple of days trying to understand why one of my local changes wasn't working. The root cause turned out to be #3324; the problem would've been instantly obvious with the changes in this PR.

Changes

This PR adapts the current error reporting we do when the compression worker fails to load so that it can be used for both the compression worker and the lazy loaded recorder module. We now use the same code in both places, so our existing infrastructure for detecting CSP-related errors can be reused.

Testing

• Local
• Staging
• Unit
• End to end

---

I have gone over the contributing documentation.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.91%. Comparing base (00f765c) to head (3026474).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/rum/src/boot/lazyLoadRecorder.ts	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3326      +/-   ##
==========================================
- Coverage   92.95%   92.91%   -0.05%     
==========================================
  Files         297      299       +2     
  Lines        7840     7851      +11     
  Branches     1785     1786       +1     
==========================================
+ Hits         7288     7295       +7     
- Misses        552      556       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sethfowler-datadog]

This helper is here for testing purposes, so that it's possible to replace the real dynamic import with a version that throws a specific error.

[BenoitZugmeyer]

Maybe something like this would be simpler to get:

export async function lazyLoadRecorder(importRecorderImpl = importRecorder) {
  try {
    return await importRecorderImpl()
  } catch {
   reportScriptLoadingError({
      error,
      source: 'Recorder',
      scriptType: 'module',
    })
  }
}

async function importRecorder() {
   const module = await import(/* webpackChunkName: "recorder" */ './startRecording')
   return module.startRecording
}

[sethfowler-datadog]

Note that the behaviors we're testing for here are the same ones we already have for the compression worker; the lazy loaded recorder module will now report errors in the same way.

[BenoitZugmeyer]

💬 suggestion: ‏ those two tests should be moved to scriptLoadingError.spec.ts or lazyLoadRecorder.spec.ts

[sethfowler-datadog]

The casing inconsistency here is a bit funky. I wanted to avoid changing the capitalization of "Worker" in case it would affect our telemetry metrics, but "Module" doesn't really feel right, so I ended up with this compromise.

[cy-moi]

🤔 I don't think capitalization would affect our telemetry metrics tbh. Same for the verb. I'm fine if we just use a same verb everyone.

[sethfowler-datadog]

Cool! In that case, I'll make things a bit more uniform.

[cit-pr-commenter]

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	147.04 KiB	147.27 KiB	231 B	+0.15%	✅
Logs	51.33 KiB	51.33 KiB	0 B	0.00%	✅
Rum Slim	105.77 KiB	105.77 KiB	0 B	0.00%	✅
Worker	24.50 KiB	24.50 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
addglobalcontext	0.002	0.002	-0.000
addaction	0.043	0.038	-0.004
addtiming	0.001	0.001	0.000
adderror	0.047	0.052	0.004
startstopsessionreplayrecording	0.008	0.015	0.007
startview	0.463	0.434	-0.029
logmessage	0.020	0.021	0.002

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫 (bytes)
addglobalcontext	28.58 KiB	27.36 KiB	-1252 B
addaction	56.59 KiB	56.35 KiB	-251 B
addtiming	26.74 KiB	27.05 KiB	312 B
adderror	59.51 KiB	57.93 KiB	-1621 B
startstopsessionreplayrecording	25.61 KiB	24.77 KiB	-867 B
startview	418.19 KiB	423.52 KiB	5.33 KiB
logmessage	60.99 KiB	62.69 KiB	1.70 KiB

🔗 RealWorld

[BenoitZugmeyer]

Maybe something like this would be simpler to get:

export async function lazyLoadRecorder(importRecorderImpl = importRecorder) {
  try {
    return await importRecorderImpl()
  } catch {
   reportScriptLoadingError({
      error,
      source: 'Recorder',
      scriptType: 'module',
    })
  }
}

async function importRecorder() {
   const module = await import(/* webpackChunkName: "recorder" */ './startRecording')
   return module.startRecording
}

[BenoitZugmeyer]

💬 suggestion: ‏ those two tests should be moved to scriptLoadingError.spec.ts or lazyLoadRecorder.spec.ts

[BenoitZugmeyer]

💬 suggestion: ‏I don't think we should report the error as telemetry, as there is high chances that we won't be able to do anything about it. Ex: the recorder fails to load because of a user connectivity issue.