fix(profiling): Guard from throwing if profiler constructor throws (#7328)

* fix(profiling): guard from throwing if profiler constructor throws

* lint

Author: JonasBa

packages/browser/src/profiling/hubextensions.ts

@@ -3,11 +3,19 @@ import type { CustomSamplingContext, Hub, Transaction, TransactionContext } from
 import { logger, uuid4 } from '@sentry/utils';
 
 import { WINDOW } from '../helpers';
-import type { JSSelfProfile, JSSelfProfiler, ProcessedJSSelfProfile } from './jsSelfProfiling';
+import type {
+  JSSelfProfile,
+  JSSelfProfiler,
+  JSSelfProfilerConstructor,
+  ProcessedJSSelfProfile,
+} from './jsSelfProfiling';
 import { sendProfile } from './sendProfile';
 
 // Max profile duration.
 const MAX_PROFILE_DURATION_MS = 30_000;
+// Keep a flag value to avoid re-initializing the profiler constructor. If it fails
+// once, it will always fail and this allows us to early return.
+let PROFILING_CONSTRUCTOR_FAILED = false;
 
 // While we experiment, per transaction sampling interval will be more flexible to work with.
 type StartTransaction = (
@@ -20,7 +28,7 @@ type StartTransaction = (
  * Check if profiler constructor is available.
  * @param maybeProfiler
  */
-function isJSProfilerSupported(maybeProfiler: unknown): maybeProfiler is typeof JSSelfProfiler {
+function isJSProfilerSupported(maybeProfiler: unknown): maybeProfiler is typeof JSSelfProfilerConstructor {
   return typeof maybeProfiler === 'function';
 }
 
@@ -49,8 +57,9 @@ export function onProfilingStartRouteTransaction(transaction: Transaction | unde
  */
 function wrapTransactionWithProfiling(transaction: Transaction): Transaction {
   // Feature support check first
-  const JSProfiler = WINDOW.Profiler;
-  if (!isJSProfilerSupported(JSProfiler)) {
+  const JSProfilerConstructor = WINDOW.Profiler;
+
+  if (!isJSProfilerSupported(JSProfilerConstructor)) {
     if (__DEBUG_BUILD__) {
       logger.log(
         '[Profiling] Profiling is not supported by this browser, Profiler interface missing on window object.',
@@ -67,6 +76,14 @@ function wrapTransactionWithProfiling(transaction: Transaction): Transaction {
     return transaction;
   }
 
+  // If constructor failed once, it will always fail, so we can early return.
+  if (PROFILING_CONSTRUCTOR_FAILED) {
+    if (__DEBUG_BUILD__) {
+      logger.log('[Profiling] Profiling has been disabled for the duration of the current user session.');
+    }
+    return transaction;
+  }
+
   const client = getCurrentHub().getClient();
   const options = client && client.getOptions();
 
@@ -91,7 +108,29 @@ function wrapTransactionWithProfiling(transaction: Transaction): Transaction {
   const samplingIntervalMS = 10;
   // Start the profiler
   const maxSamples = Math.floor(MAX_PROFILE_DURATION_MS / samplingIntervalMS);
-  const profiler = new JSProfiler({ sampleInterval: samplingIntervalMS, maxBufferSize: maxSamples });
+  let profiler: JSSelfProfiler | undefined;
+
+  // Attempt to initialize the profiler constructor, if it fails, we disable profiling for the current user session.
+  // This is likely due to a missing 'Document-Policy': 'js-profiling' header. We do not want to throw an error if this happens
+  // as we risk breaking the user's application, so just disable profiling and log an error.
+  try {
+    profiler = new JSProfilerConstructor({ sampleInterval: samplingIntervalMS, maxBufferSize: maxSamples });
+  } catch (e) {
+    if (__DEBUG_BUILD__) {
+      logger.log(
+        "[Profiling] Failed to initialize the Profiling constructor, this is likely due to a missing 'Document-Policy': 'js-profiling' header.",
+      );
+      logger.log('[Profiling] Disabling profiling for current user session.');
+    }
+    PROFILING_CONSTRUCTOR_FAILED = true;
+  }
+
+  // We failed to construct the profiler, fallback to original transaction - there is no need to log
+  // anything as we already did that in the try/catch block.
+  if (!profiler) {
+    return transaction;
+  }
+
   if (__DEBUG_BUILD__) {
     logger.log(`[Profiling] started profiling transaction: ${transaction.name || transaction.description}`);
   }
@@ -118,6 +157,10 @@ function wrapTransactionWithProfiling(transaction: Transaction): Transaction {
     if (!transaction) {
       return;
     }
+    // Satisfy the type checker, but profiler will always be defined here.
+    if (!profiler) {
+      return;
+    }
     if (processedProfile) {
       if (__DEBUG_BUILD__) {
         logger.log(

packages/browser/src/profiling/jsSelfProfiling.ts

@@ -32,21 +32,21 @@ export interface ProcessedJSSelfProfile extends JSSelfProfile {
 
 type BufferFullCallback = (trace: JSSelfProfile) => void;
 
-interface JSSelfProfiler {
+export interface JSSelfProfiler {
   sampleInterval: number;
   stopped: boolean;
 
   stop: () => Promise<JSSelfProfile>;
   addEventListener(event: 'samplebufferfull', callback: BufferFullCallback): void;
 }
 
-export declare const JSSelfProfiler: {
+export declare const JSSelfProfilerConstructor: {
   new (options: { sampleInterval: number; maxBufferSize: number }): JSSelfProfiler;
 };
 
 declare global {
   interface Window {
-    Profiler: typeof JSSelfProfiler | undefined;
+    Profiler: typeof JSSelfProfilerConstructor | undefined;
   }
 }
 

packages/browser/test/unit/profiling/hubextensions.test.ts

@@ -0,0 +1,91 @@
+import { TextDecoder, TextEncoder } from 'util';
+// @ts-ignore patch the encoder on the window, else importing JSDOM fails (deleted in afterAll)
+const patchedEncoder = (!global.window.TextEncoder && (global.window.TextEncoder = TextEncoder)) || true;
+// @ts-ignore patch the encoder on the window, else importing JSDOM fails (deleted in afterAll)
+const patchedDecoder = (!global.window.TextDecoder && (global.window.TextDecoder = TextDecoder)) || true;
+
+import { getCurrentHub } from '@sentry/core';
+import type { Transaction } from '@sentry/types';
+import { JSDOM } from 'jsdom';
+
+import { onProfilingStartRouteTransaction } from '../../../src';
+
+// @ts-ignore store a reference so we can reset it later
+const globalDocument = global.document;
+// @ts-ignore store a reference so we can reset it later
+const globalWindow = global.window;
+// @ts-ignore store a reference so we can reset it later
+const globalLocation = global.location;
+
+describe('BrowserProfilingIntegration', () => {
+  beforeEach(() => {
+    const dom = new JSDOM();
+    // @ts-ignore need to override global document
+    global.document = dom.window.document;
+    // @ts-ignore need to override global document
+    global.window = dom.window;
+    // @ts-ignore need to override global document
+    global.location = dom.window.location;
+
+    const hub = getCurrentHub();
+    const client: any = {
+      getDsn() {
+        return {};
+      },
+      getTransport() {
+        return {
+          send() {},
+        };
+      },
+      getOptions() {
+        return {
+          profilesSampleRate: 1,
+        };
+      },
+    };
+
+    hub.bindClient(client);
+  });
+
+  // Reset back to previous values
+  afterEach(() => {
+    // @ts-ignore need to override global document
+    global.document = globalDocument;
+    // @ts-ignore need to override global document
+    global.window = globalWindow;
+    // @ts-ignore need to override global document
+    global.location = globalLocation;
+  });
+  afterAll(() => {
+    // @ts-ignore patch the encoder on the window, else importing JSDOM fails
+    patchedEncoder && delete global.window.TextEncoder;
+    // @ts-ignore patch the encoder on the window, else importing JSDOM fails
+    patchedDecoder && delete global.window.TextDecoder;
+  });
+
+  it('does not throw if Profiler is not available', () => {
+    // @ts-ignore force api to be undefined
+    global.window.Profiler = undefined;
+    // set sampled to true so that profiling does not early return
+    const mockTransaction = { sampled: true } as Transaction;
+    expect(() => onProfilingStartRouteTransaction(mockTransaction)).not.toThrow();
+  });
+  it('does not throw if constructor throws', () => {
+    const spy = jest.fn();
+
+    class Profiler {
+      constructor() {
+        spy();
+        throw new Error('Profiler constructor error');
+      }
+    }
+
+    // set sampled to true so that profiling does not early return
+    const mockTransaction = { sampled: true } as Transaction;
+
+    // @ts-ignore override with our own constructor
+    global.window.Profiler = Profiler;
+    expect(() => onProfilingStartRouteTransaction(mockTransaction)).not.toThrow();
+    expect(spy).toHaveBeenCalled();
+  });
+});