Finialize test and make handler synchronous

Author: aliu39

dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/init.js

@@ -11,7 +11,7 @@ Sentry.init({
   integrations: [window.LDIntegration],
 });
 
-window.MockLaunchDarkly = {
+const MockLaunchDarkly = {
   initialize(_clientId, context, options) {
     const flagUsedHandler = (options && options.inspectors) ? options.inspectors[0].method : undefined;
 
@@ -27,7 +27,7 @@ window.MockLaunchDarkly = {
 };
 
 window.InitializeLD = () => {
-  return window.MockLaunchDarkly.initialize(
+  return MockLaunchDarkly.initialize(
     'example-client-id',
     { kind: 'user', key: 'example-context-key' },
     { inspectors: [buildLaunchDarklyFlagUsedHandler()] },

dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/test.ts

@@ -3,40 +3,9 @@ import { expect } from '@playwright/test';
 import { sentryTest } from '../../../../utils/fixtures';
 
 import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
-import type { LDContext, LDOptions, LDFlagValue, LDClient, LDEvaluationDetail } from 'launchdarkly-js-client-sdk';
-import type { Event } from '@sentry/types';
-
-// const MockLaunchDarkly = { //TODO: remove in favor of window.MockLaunchDarkly from init.js
-//   initialize(
-//     _clientId: string,
-//     context: LDContext,
-//     options: LDOptions,
-//   ) {
-//     const flagUsedHandler = options?.inspectors?.[0].method;
-//     const wellTypedHandler = flagUsedHandler as ((
-//       flagKey: string,
-//       flagDetail: LDEvaluationDetail,
-//       context: LDContext,
-//     ) => void) | undefined;
-
-//     return {
-//       variation(key: string, defaultValue: LDFlagValue) {
-//         wellTypedHandler?.(key, { value: defaultValue }, context);
-//         return defaultValue;
-//       },
-//     };
-//   },
-// };
 
 sentryTest('e2e test', async ({ getLocalTestPath, page }) => {
-  let errorEventId: string = 'invalid_id';
   await page.route('https://dsn.ingest.sentry.io/**/*', route => {
-    const event = envelopeRequestParser(route.request());
-    // error events have no type field
-    if (event && !event.type && event.event_id) {
-      errorEventId = event.event_id;
-    }
-
     return route.fulfill({
       status: 200,
       contentType: 'application/json',
@@ -47,30 +16,27 @@ sentryTest('e2e test', async ({ getLocalTestPath, page }) => {
   const url = await getLocalTestPath({ testDir: __dirname, skipDsnRouteHandler: true });
   await page.goto(url);
 
-  // TODO: could this be in init.js?
   await page.waitForFunction(() => {
     const ldClient = (window as any).InitializeLD();
-    ldClient.variation('feat1', false);
-    ldClient.variation('feat2', false);
-    ldClient.variation('feat3', false);
-    ldClient.variation('feat2', true);
+    for (let i = 1; i <= 100; i++) { // TODO: import constant for buffer size
+      ldClient.variation(`feat${i}`, false);
+    }
+    ldClient.variation('feat101', true); // eviction
+    ldClient.variation('feat3', true);   // update
     return true;
   });
 
-
-  // TODO: eviction not tested
-
-  // trigger error
   const reqPromise = waitForErrorRequest(page);
   await page.locator('#error').click();
   const req = await reqPromise;
-
-  // console.log(errorEventId);
   const event = envelopeRequestParser(req);
 
-  expect(event.contexts?.flags?.values).toEqual([
-    { flag: 'feat1', result: false },
-    { flag: 'feat3', result: false },
-    { flag: 'feat2', result: true },
-  ]);
+  const expectedFlags = [{ flag: 'feat2', result: false }];
+  for (let i = 4; i <= 100; i++) {
+    expectedFlags.push({ flag: `feat${i}`, result: false });
+  }
+  expectedFlags.push({ flag: 'feat101', result: true });
+  expectedFlags.push({ flag: 'feat3', result: true });
+
+  expect(event.contexts?.flags?.values).toEqual(expectedFlags);
 });

packages/browser/src/integrations/launchdarkly.ts

@@ -47,9 +47,7 @@ export function buildLaunchDarklyFlagUsedHandler(): LDInspectionFlagUsedHandler
     name: 'sentry-flag-auditor',
     type: 'flag-used',
 
-    // We don't want the handler to impact the performance of the user's flag evaluations.
-    synchronous: false, // TODO: this could lead to race conditions where an error directly after an eval might not contain the eval
-    // TODO: the flag buffer itself isn't thread-safe, yet this handler and the event processor could access it at the same time.
+    synchronous: true,
 
     /**
      * Handle a flag evaluation by storing its name and value on the current scope.
@@ -62,6 +60,7 @@ export function buildLaunchDarklyFlagUsedHandler(): LDInspectionFlagUsedHandler
         }
         const flagBuffer = scopeContexts.flags.values;
         insertToFlagBuffer(flagBuffer, flagKey, flagDetail.value);
+        // TODO: the flag buffer isn't thread-safe, yet this handler and the event processor could access it at the same time.
       }
       return;
     },