fix: Added extra check for undefined values (#260)

Cdub-9 · web-flow · commit ded5449af7d9 · 2024-02-23T13:12:21.000-08:00
diff --git a/__tests__/steps/measureStep.spec.ts b/__tests__/steps/measureStep.spec.ts
@@ -139,14 +139,44 @@ describe('measureStep', () => {
         },
       });
     });
-    it('should return when the start mark doesnt exist', () => { 
+    it('should return when the start mark doesnt exist', () => {
       measureSpy = jest.spyOn(WP, 'measure');
       // ============ Mock Data ============
       jest.spyOn(WP, 'getEntriesByName').mockImplementationOnce(name => {
         return [];
       });
       measureStep('not-valid-step', 'startMark', 'endmark');
       expect(measureSpy).toBeCalledTimes(0);
-    })
+    });
+
+    it('should return when the start mark doesnt exist', () => {
+      measureSpy = jest.spyOn(WP, 'measure');
+      jest
+        .spyOn(WP, 'measure')
+        // @ts-ignore
+        .mockImplementationOnce(() => undefined);
+      // ============ Mock Data ============
+      jest.spyOn(WP, 'getEntriesByName').mockImplementationOnce(name => {
+        const entries: Record<string, PerformanceEntry> = {
+          'mark.start_navigate_to_second_screen_first_journey': {
+            name: 'mark.start_navigate_to_second_screen_first_journey',
+            entryType: 'mark',
+            duration: 0,
+            startTime: 0,
+            toJSON: jest.fn(),
+          },
+          'mark.loaded_second_screen_first_journey': {
+            name: 'mark.loaded_second_screen_first_journey',
+            entryType: 'mark',
+            duration: 0,
+            startTime: 100,
+            toJSON: jest.fn(),
+          },
+        };
+        return [entries[name]] ?? [];
+      });
+      measureStep('load_second_screen_first_journey', 'start_navigate_to_second_screen_first_journey', 'start_navigate_to_second_screen_first_journey');
+      expect(measureSpy).toBeCalledTimes(1); // after the first call that returns undefined, it exits the function and does not call measure again
+    });
   });
 });
diff --git a/src/steps/measureStep.ts b/src/steps/measureStep.ts
@@ -12,20 +12,38 @@ export const measureStep = (
   const stepMetricName = S + step;
   const startMarkExists = WP.getEntriesByName(M + startMark).length > 0;
   const endMarkExists = WP.getEntriesByName(M + endMark).length > 0;
-  if (!endMarkExists || !startMarkExists || !config.steps || !config.steps[step]) {
+  if (
+    !endMarkExists ||
+    !startMarkExists ||
+    !config.steps ||
+    !config.steps[step]
+  ) {
     return;
   }
 
   const { maxOutlierThreshold, vitalsThresholds } =
     STEP_THRESHOLDS[config.steps[step].threshold];
 
   const stepMeasure = WP.measure(stepMetricName, M + startMark, M + endMark);
+
+  // checking to ensure stepMeasure is defined - it can be undefined
+  // if measure is called on a mark that has already been measured and cleared
+  if (!stepMeasure) {
+    return;
+  }
+
   const { duration } = stepMeasure;
   if (duration <= maxOutlierThreshold) {
     const score = getRating(duration, vitalsThresholds);
     // Do not want to measure or log negative metrics
     if (duration >= 0) {
-      reportPerf('userJourneyStep', duration, score, { stepName: step }, undefined);
+      reportPerf(
+        'userJourneyStep',
+        duration,
+        score,
+        { stepName: step },
+        undefined,
+      );
       WP.measure(`step.${step}_vitals_${score}`, {
         start: stepMeasure.startTime + stepMeasure.duration,
         end: stepMeasure.startTime + stepMeasure.duration,
diff --git a/src/steps/measureSteps.ts b/src/steps/measureSteps.ts
@@ -16,10 +16,14 @@ export const measureSteps = (mark: string) => {
     Object.keys(finalSteps).forEach(startMark => {
       const possibleSteps = finalSteps[startMark];
       possibleSteps.forEach(removeActiveStep);
+      // Async processing all possible steps 
+      // needs to be async due to clearing previously measured steps and marks.
+      // If we run all concurrently, there is a chance for a race condition 
+      // where we are adding and deleteing the entries in WP.Performance which caused the measure to fail.
       Promise.all(
-        possibleSteps.map(step => {
+        possibleSteps.map(async step => {
           // measure
-          measureStep(step, startMark, mark);
+          await measureStep(step, startMark, mark);
         }),
       ).catch(() => {
         // TODO @zizzamia log error
