From 4fa98c7b78dd5d824135a2cd6f1f1f0657951690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 5 Sep 2023 10:59:07 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=8A=20[RUM-253]=20customize=20deflate?= =?UTF-8?q?=20worker=20failure=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/boot/recorderApi.ts | 1 + packages/rum/src/boot/startRecording.spec.ts | 2 +- .../src/domain/deflate/deflateWorker.spec.ts | 129 ++++++++++-------- .../rum/src/domain/deflate/deflateWorker.ts | 25 ++-- 4 files changed, 91 insertions(+), 66 deletions(-) diff --git a/packages/rum/src/boot/recorderApi.ts b/packages/rum/src/boot/recorderApi.ts index 751b4d684a..74133ea479 100644 --- a/packages/rum/src/boot/recorderApi.ts +++ b/packages/rum/src/boot/recorderApi.ts @@ -126,6 +126,7 @@ export function makeRecorderApi( const worker = startDeflateWorker( configuration, + 'Datadog Session Replay', () => { stopStrategy() }, diff --git a/packages/rum/src/boot/startRecording.spec.ts b/packages/rum/src/boot/startRecording.spec.ts index 2069bb7af0..ee9d347763 100644 --- a/packages/rum/src/boot/startRecording.spec.ts +++ b/packages/rum/src/boot/startRecording.spec.ts @@ -47,7 +47,7 @@ describe('startRecording', () => { textField = document.createElement('input') sandbox.appendChild(textField) - const worker = startDeflateWorker(configuration, noop) + const worker = startDeflateWorker(configuration, 'Datadog Session Replay', noop) setupBuilder = setup() .withViewContexts({ diff --git a/packages/rum/src/domain/deflate/deflateWorker.spec.ts b/packages/rum/src/domain/deflate/deflateWorker.spec.ts index a99a1281cc..a881352f3d 100644 --- a/packages/rum/src/domain/deflate/deflateWorker.spec.ts +++ b/packages/rum/src/domain/deflate/deflateWorker.spec.ts @@ -1,5 +1,5 @@ import type { RawTelemetryEvent } from '@datadog/browser-core' -import { display, isIE, noop, resetTelemetry, startFakeTelemetry } from '@datadog/browser-core' +import { display, isIE, resetTelemetry, startFakeTelemetry } from '@datadog/browser-core' import type { RumConfiguration } from '@datadog/browser-rum-core' import type { Clock } from '@datadog/browser-core/test' import { mockClock } from '@datadog/browser-core/test' @@ -14,10 +14,23 @@ describe('startDeflateWorker', () => { let mockWorker: MockWorker let createDeflateWorkerSpy: jasmine.Spy let onInitializationFailureSpy: jasmine.Spy<() => void> - let configuration: RumConfiguration + + function startDeflateWorkerWithDefaults({ + configuration = {}, + source = 'Datadog Session Replay', + }: { + configuration?: Partial + source?: string + } = {}) { + return startDeflateWorker( + configuration as RumConfiguration, + source, + onInitializationFailureSpy, + createDeflateWorkerSpy + ) + } beforeEach(() => { - configuration = {} as RumConfiguration mockWorker = new MockWorker() onInitializationFailureSpy = jasmine.createSpy('onInitializationFailureSpy') createDeflateWorkerSpy = jasmine.createSpy('createDeflateWorkerSpy').and.callFake(() => mockWorker) @@ -28,7 +41,7 @@ describe('startDeflateWorker', () => { }) it('creates a deflate worker', () => { - const worker = startDeflateWorker(configuration, onInitializationFailureSpy, createDeflateWorkerSpy) + const worker = startDeflateWorkerWithDefaults() expect(createDeflateWorkerSpy).toHaveBeenCalledTimes(1) expect(worker).toBe(mockWorker) @@ -37,17 +50,17 @@ describe('startDeflateWorker', () => { }) it('uses the previously created worker during loading', () => { - const worker1 = startDeflateWorker(configuration, noop, createDeflateWorkerSpy) - const worker2 = startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + const worker1 = startDeflateWorkerWithDefaults() + const worker2 = startDeflateWorkerWithDefaults() expect(createDeflateWorkerSpy).toHaveBeenCalledTimes(1) expect(worker1).toBe(worker2) }) it('uses the previously created worker once initialized', () => { - const worker1 = startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + const worker1 = startDeflateWorkerWithDefaults() mockWorker.processAllMessages() - const worker2 = startDeflateWorker(configuration, onInitializationFailureSpy, createDeflateWorkerSpy) + const worker2 = startDeflateWorkerWithDefaults() expect(createDeflateWorkerSpy).toHaveBeenCalledTimes(1) expect(worker1).toBe(worker2) @@ -60,13 +73,11 @@ describe('startDeflateWorker', () => { // mimic Chrome behavior let CSP_ERROR: DOMException let displaySpy: jasmine.Spy - let configuration: RumConfiguration beforeEach(() => { if (isIE()) { pending('IE does not support CSP blocking worker creation') } - configuration = {} as RumConfiguration displaySpy = spyOn(display, 'error') telemetryEvents = startFakeTelemetry() CSP_ERROR = new DOMException( @@ -80,40 +91,37 @@ describe('startDeflateWorker', () => { describe('Chrome and Safari behavior: exception during worker creation', () => { it('returns undefined when the worker creation throws an exception', () => { - const worker = startDeflateWorker(configuration, noop, () => { - throw CSP_ERROR - }) + createDeflateWorkerSpy.and.throwError(CSP_ERROR) + const worker = startDeflateWorkerWithDefaults() expect(worker).toBeUndefined() }) it('displays CSP instructions when the worker creation throws a CSP error', () => { - startDeflateWorker(configuration, noop, () => { - throw CSP_ERROR - }) + createDeflateWorkerSpy.and.throwError(CSP_ERROR) + startDeflateWorkerWithDefaults() expect(displaySpy).toHaveBeenCalledWith( jasmine.stringContaining('Please make sure CSP is correctly configured') ) }) it('does not report CSP errors to telemetry', () => { - startDeflateWorker(configuration, noop, () => { - throw CSP_ERROR - }) + createDeflateWorkerSpy.and.throwError(CSP_ERROR) + startDeflateWorkerWithDefaults() expect(telemetryEvents).toEqual([]) }) it('does not try to create a worker again after the creation failed', () => { - startDeflateWorker(configuration, noop, () => { - throw CSP_ERROR - }) - startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + createDeflateWorkerSpy.and.throwError(CSP_ERROR) + startDeflateWorkerWithDefaults() + createDeflateWorkerSpy.calls.reset() + startDeflateWorkerWithDefaults() expect(createDeflateWorkerSpy).not.toHaveBeenCalled() }) }) describe('Firefox behavior: error during worker loading', () => { it('displays ErrorEvent as CSP error', () => { - startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults() mockWorker.dispatchErrorEvent() expect(displaySpy).toHaveBeenCalledWith( jasmine.stringContaining('Please make sure CSP is correctly configured') @@ -121,24 +129,28 @@ describe('startDeflateWorker', () => { }) it('calls the initialization failure callback when of an error occurs during loading', () => { - startDeflateWorker(configuration, onInitializationFailureSpy, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults() mockWorker.dispatchErrorEvent() expect(onInitializationFailureSpy).toHaveBeenCalledTimes(1) }) it('returns undefined if an error occurred in a previous loading', () => { - startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults() mockWorker.dispatchErrorEvent() + onInitializationFailureSpy.calls.reset() - const worker = startDeflateWorker(configuration, onInitializationFailureSpy, createDeflateWorkerSpy) + const worker = startDeflateWorkerWithDefaults() expect(worker).toBeUndefined() expect(onInitializationFailureSpy).not.toHaveBeenCalled() }) it('adjusts the error message when a workerUrl is set', () => { - configuration.workerUrl = '/worker.js' - startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults({ + configuration: { + workerUrl: '/worker.js', + }, + }) mockWorker.dispatchErrorEvent() expect(displaySpy).toHaveBeenCalledWith( jasmine.stringContaining( @@ -148,25 +160,25 @@ describe('startDeflateWorker', () => { }) it('calls all registered callbacks when the worker initialization fails', () => { - const onInitializationFailureSpy1 = jasmine.createSpy() - const onInitializationFailureSpy2 = jasmine.createSpy() - startDeflateWorker(configuration, onInitializationFailureSpy1, createDeflateWorkerSpy) - startDeflateWorker(configuration, onInitializationFailureSpy2, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults() + startDeflateWorkerWithDefaults() mockWorker.dispatchErrorEvent() - expect(onInitializationFailureSpy1).toHaveBeenCalledTimes(1) - expect(onInitializationFailureSpy2).toHaveBeenCalledTimes(1) + expect(onInitializationFailureSpy).toHaveBeenCalledTimes(2) }) }) }) describe('initialization timeout', () => { let displaySpy: jasmine.Spy - let configuration: RumConfiguration let clock: Clock beforeEach(() => { - configuration = {} as RumConfiguration displaySpy = spyOn(display, 'error') + createDeflateWorkerSpy.and.callFake( + () => + // Creates a worker that does nothing + new Worker(URL.createObjectURL(new Blob(['']))) + ) clock = mockClock() }) @@ -175,16 +187,18 @@ describe('startDeflateWorker', () => { }) it('displays an error message when the worker does not respond to the init action', () => { - startDeflateWorker( - configuration, - noop, - () => - // Creates a worker that does nothing - new Worker(URL.createObjectURL(new Blob(['']))) + startDeflateWorkerWithDefaults() + clock.tick(INITIALIZATION_TIME_OUT_DELAY) + expect(displaySpy).toHaveBeenCalledOnceWith( + 'Datadog Session Replay failed to start: a timeout occurred while initializing the Worker' ) + }) + + it('displays a customized error message', () => { + startDeflateWorkerWithDefaults({ source: 'Foo' }) clock.tick(INITIALIZATION_TIME_OUT_DELAY) expect(displaySpy).toHaveBeenCalledOnceWith( - 'Session Replay recording failed to start: a timeout occurred while initializing the Worker' + 'Foo failed to start: a timeout occurred while initializing the Worker' ) }) }) @@ -193,10 +207,8 @@ describe('startDeflateWorker', () => { let telemetryEvents: RawTelemetryEvent[] const UNKNOWN_ERROR = new Error('boom') let displaySpy: jasmine.Spy - let configuration: RumConfiguration beforeEach(() => { - configuration = {} as RumConfiguration displaySpy = spyOn(display, 'error') telemetryEvents = startFakeTelemetry() }) @@ -206,19 +218,26 @@ describe('startDeflateWorker', () => { }) it('displays an error message when the worker creation throws an unknown error', () => { - startDeflateWorker(configuration, noop, () => { - throw UNKNOWN_ERROR - }) + createDeflateWorkerSpy.and.throwError(UNKNOWN_ERROR) + startDeflateWorkerWithDefaults() expect(displaySpy).toHaveBeenCalledOnceWith( - 'Session Replay recording failed to start: an error occurred while creating the Worker:', + 'Datadog Session Replay failed to start: an error occurred while creating the Worker:', + UNKNOWN_ERROR + ) + }) + + it('displays a customized error message', () => { + createDeflateWorkerSpy.and.throwError(UNKNOWN_ERROR) + startDeflateWorkerWithDefaults({ source: 'Foo' }) + expect(displaySpy).toHaveBeenCalledOnceWith( + 'Foo failed to start: an error occurred while creating the Worker:', UNKNOWN_ERROR ) }) it('reports unknown errors to telemetry', () => { - startDeflateWorker(configuration, noop, () => { - throw UNKNOWN_ERROR - }) + createDeflateWorkerSpy.and.throwError(UNKNOWN_ERROR) + startDeflateWorkerWithDefaults() expect(telemetryEvents).toEqual([ { type: 'log', @@ -230,13 +249,13 @@ describe('startDeflateWorker', () => { }) it('does not display error messages as CSP error', () => { - startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults() mockWorker.dispatchErrorMessage('foo') expect(displaySpy).not.toHaveBeenCalledWith(jasmine.stringContaining('CSP')) }) it('reports errors occurring after loading to telemetry', () => { - startDeflateWorker(configuration, noop, createDeflateWorkerSpy) + startDeflateWorkerWithDefaults() mockWorker.processAllMessages() mockWorker.dispatchErrorMessage('boom', TEST_STREAM_ID) diff --git a/packages/rum/src/domain/deflate/deflateWorker.ts b/packages/rum/src/domain/deflate/deflateWorker.ts index 197e710cf2..b5b69c251d 100644 --- a/packages/rum/src/domain/deflate/deflateWorker.ts +++ b/packages/rum/src/domain/deflate/deflateWorker.ts @@ -52,12 +52,13 @@ let state: DeflateWorkerState = { status: DeflateWorkerStatus.Nil } export function startDeflateWorker( configuration: RumConfiguration, + source: string, onInitializationFailure: () => void, createDeflateWorkerImpl = createDeflateWorker ) { if (state.status === DeflateWorkerStatus.Nil) { // doStartDeflateWorker updates the state to "loading" or "error" - doStartDeflateWorker(configuration, createDeflateWorkerImpl) + doStartDeflateWorker(configuration, source, createDeflateWorkerImpl) } switch (state.status) { @@ -89,11 +90,15 @@ export function getDeflateWorkerStatus() { * * more details: https://bugzilla.mozilla.org/show_bug.cgi?id=1736865#c2 */ -export function doStartDeflateWorker(configuration: RumConfiguration, createDeflateWorkerImpl = createDeflateWorker) { +export function doStartDeflateWorker( + configuration: RumConfiguration, + source: string, + createDeflateWorkerImpl = createDeflateWorker +) { try { const worker = createDeflateWorkerImpl(configuration) const { stop: removeErrorListener } = addEventListener(configuration, worker, 'error', (error) => { - onError(configuration, error) + onError(configuration, source, error) }) const { stop: removeMessageListener } = addEventListener( configuration, @@ -101,14 +106,14 @@ export function doStartDeflateWorker(configuration: RumConfiguration, createDefl 'message', ({ data }: MessageEvent) => { if (data.type === 'errored') { - onError(configuration, data.error, data.streamId) + onError(configuration, source, data.error, data.streamId) } else if (data.type === 'initialized') { onInitialized(data.version) } } ) worker.postMessage({ action: 'init' }) - setTimeout(onTimeout, INITIALIZATION_TIME_OUT_DELAY) + setTimeout(() => onTimeout(source), INITIALIZATION_TIME_OUT_DELAY) const stop = () => { removeErrorListener() removeMessageListener() @@ -116,13 +121,13 @@ export function doStartDeflateWorker(configuration: RumConfiguration, createDefl state = { status: DeflateWorkerStatus.Loading, worker, stop, initializationFailureCallbacks: [] } } catch (error) { - onError(configuration, error) + onError(configuration, source, error) } } -function onTimeout() { +function onTimeout(source: string) { if (state.status === DeflateWorkerStatus.Loading) { - display.error('Session Replay recording failed to start: a timeout occurred while initializing the Worker') + display.error(`${source} failed to start: a timeout occurred while initializing the Worker`) state.initializationFailureCallbacks.forEach((callback) => callback()) state = { status: DeflateWorkerStatus.Error } } @@ -134,9 +139,9 @@ function onInitialized(version: string) { } } -function onError(configuration: RumConfiguration, error: unknown, streamId?: number) { +function onError(configuration: RumConfiguration, source: string, error: unknown, streamId?: number) { if (state.status === DeflateWorkerStatus.Loading || state.status === DeflateWorkerStatus.Nil) { - display.error('Session Replay recording failed to start: an error occurred while creating the Worker:', error) + display.error(`${source} failed to start: an error occurred while creating the Worker:`, error) if (error instanceof Event || (error instanceof Error && isMessageCspRelated(error.message))) { let baseMessage if (configuration.workerUrl) {