fix: skip generating error events for unauthorized or forbidden errors (

#188)
bucketeer-io · Nov 26, 2024 · 0c48090 · 0c48090
1 parent d044303
commit 0c48090
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 38 deletions.
diff --git a/e2e/events.spec.ts b/e2e/events.spec.ts
@@ -198,7 +198,7 @@ suite('e2e/events', () => {
 
       const events = component.dataModule.eventStorage().getAll()
 
-      expect(events).toHaveLength(1)
+      expect(events).toHaveLength(0)
       expect(
         events.some((e) => {
           return (
@@ -207,16 +207,14 @@ suite('e2e/events', () => {
             e.event.event.apiId === ApiId.GET_EVALUATIONS
           )
         }),
-      ).toBe(true)
+      ).toBe(false)
 
-      await expect(() => client.flush()).rejects.toThrowError(
-        ForbiddenException,
-      )
+      await client.flush()
 
       const events2 = component.dataModule.eventStorage().getAll()
 
-      // error from /register_events does not get stored
-      expect(events2).toHaveLength(1)
+      // no events should be stored
+      expect(events2).toHaveLength(0)
 
       destroyBKTClient()
 
@@ -236,10 +234,9 @@ suite('e2e/events', () => {
       const component2 = getDefaultComponent(client)
 
       const events3 = component2.dataModule.eventStorage().getAll()
-
-      // error from /register_events does not get stored
-      expect(events3).toHaveLength(3)
-      // ForbiddenError should still exist
+      // 2 events - latency and response size
+      expect(events3).toHaveLength(2)
+      // ForbiddenError should not exist
       expect(
         events.some((e) => {
           return (
@@ -248,7 +245,7 @@ suite('e2e/events', () => {
             e.event.event.apiId === ApiId.GET_EVALUATIONS
           )
         }),
-      ).toBe(true)
+      ).toBe(false)
 
       await client2.flush()
 

diff --git a/src/internal/event/EventInteractor.ts b/src/internal/event/EventInteractor.ts
@@ -3,7 +3,7 @@ import { Clock } from '../Clock'
 import { IdGenerator } from '../IdGenerator'
 import { Evaluation } from '../model/Evaluation'
 import { EventType, Event, MetricsEvent } from '../model/Event'
-import { ApiId } from '../model/MetricsEventData'
+import { ApiId, MetricsEventType } from '../model/MetricsEventData'
 import { User } from '../model/User'
 import { ApiClient } from '../remote/ApiClient'
 import {
@@ -169,6 +169,17 @@ export class EventInteractor {
   }
 
   trackFailure(apiId: ApiId, featureTag: string, error: BKTException): void {
+
+    if (error.type === MetricsEventType.UnauthorizedError) {
+      console.error('An unauthorized error occurred. Please check your API Key.')
+      return
+    }
+
+    if (error.type === MetricsEventType.ForbiddenError) {
+      console.error('An forbidden error occurred. Please check your API Key.')
+      return
+    }
+
     const metricsEvents: Event[] = [
       {
         id: this.idGenerator.newId(),

diff --git a/test/internal/evaluation/EvaluationInteractor.spec.ts b/test/internal/evaluation/EvaluationInteractor.spec.ts
@@ -1,4 +1,4 @@
-import { HttpResponse, StrictRequest, http } from 'msw'
+import { HttpResponse, http } from 'msw'
 import { SetupServer } from 'msw/node'
 import {
   expect,
@@ -97,7 +97,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
         component.dataModule.evaluationStorage().getCurrentEvaluationsId(),
       ).toBeNull()
 
-      const mockListener = vi.fn<[], void>()
+      const mockListener = vi.fn()
       interactor.addUpdateListener(mockListener)
 
       const result = await interactor.fetch(user1)
@@ -158,7 +158,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
         }, { once: true }),
       )
 
-      const mockListener = vi.fn<[], void>()
+      const mockListener = vi.fn()
       interactor.addUpdateListener(mockListener)
 
       // initial request
@@ -194,10 +194,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
     })
 
     test('update with no change', async () => {
-      const requestInterceptor = vi.fn<
-        [StrictRequest<GetEvaluationsRequest>],
-        void
-      >()
+      const requestInterceptor = vi.fn()
 
       server.use(
         http.post<
@@ -233,7 +230,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
         }),
       )
 
-      const mockListener = vi.fn<[], void>()
+      const mockListener = vi.fn()
       interactor.addUpdateListener(mockListener)
 
       const result1 = await interactor.fetch(user1)
@@ -360,7 +357,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
         userAttributesUpdated: false,
       })
 
-      const mockListener = vi.fn<[], void>()
+      const mockListener = vi.fn()
 
       interactor.addUpdateListener(mockListener)
 
@@ -414,7 +411,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
         userAttributesUpdated: false,
       })
 
-      const mockListener = vi.fn<[], void>()
+      const mockListener = vi.fn()
 
       interactor.addUpdateListener(mockListener)
 
@@ -468,7 +465,7 @@ suite('internal/evaluation/EvaluationInteractor', () => {
         userAttributesUpdated: false,
       })
 
-      const mockListener = vi.fn<[], void>()
+      const mockListener = vi.fn()
 
       interactor.addUpdateListener(mockListener)
 

diff --git a/test/internal/event/EventInteractor.spec.ts b/test/internal/event/EventInteractor.spec.ts
@@ -15,10 +15,12 @@ import {
 import { BKTConfig, defineBKTConfig } from '../../../src/BKTConfig'
 import {
   BadRequestException,
+  ForbiddenException,
   InternalServerErrorException,
   NetworkException,
   RedirectRequestException,
   TimeoutException,
+  UnauthorizedException,
   UnknownException,
 } from '../../../src/BKTExceptions'
 import { DefaultComponent } from '../../../src/internal/di/Component'
@@ -106,7 +108,7 @@ suite('internal/event/EventInteractor', () => {
   })
 
   test('trackEvaluationEvent', () => {
-    const mockListener = vi.fn<[Event[]], void>()
+    const mockListener = vi.fn()
     interactor.setEventUpdateListener(mockListener)
 
     interactor.trackEvaluationEvent('feature_tag_value', user1, evaluation1)
@@ -144,7 +146,7 @@ suite('internal/event/EventInteractor', () => {
   })
 
   test('trackDefaultEvaluationEvent', () => {
-    const mockListener = vi.fn<[Event[]], void>()
+    const mockListener = vi.fn()
     interactor.setEventUpdateListener(mockListener)
 
     interactor.trackDefaultEvaluationEvent(
@@ -186,7 +188,7 @@ suite('internal/event/EventInteractor', () => {
   })
 
   test('trackGoalEvent', () => {
-    const mockListener = vi.fn<[Event[]], void>()
+    const mockListener = vi.fn()
     interactor.setEventUpdateListener(mockListener)
 
     interactor.trackGoalEvent('feature_tag_value', user1, 'goal_id_value', 0.5)
@@ -220,7 +222,7 @@ suite('internal/event/EventInteractor', () => {
   })
 
   test('trackSuccess', () => {
-    const mockListener = vi.fn<[Event[]], void>()
+    const mockListener = vi.fn()
     interactor.setEventUpdateListener(mockListener)
 
     interactor.trackSuccess(ApiId.GET_EVALUATION, 'feature_tag_value', 1, 723)
@@ -308,7 +310,7 @@ suite('internal/event/EventInteractor', () => {
       extraLabels: { timeout: '1.5' },
     },
   ])('trackFailure: $errr -> type: $type', ({ error, type, extraLabels }) => {
-    const mockListener = vi.fn<[Event[]], void>()
+    const mockListener = vi.fn()
     interactor.setEventUpdateListener(mockListener)
 
     interactor.trackFailure(ApiId.GET_EVALUATION, 'feature_tag_value', error)
@@ -402,14 +404,41 @@ suite('internal/event/EventInteractor', () => {
     ])
   })
 
+  test('Skip generating error events for unauthorized or forbidden errors', () => {
+    // trackFailure saves one Event in each call
+    interactor.trackFailure(
+      ApiId.GET_EVALUATIONS,
+      'feature_tag_value',
+      new UnauthorizedException(),
+    )
+    interactor.trackFailure(
+      ApiId.GET_EVALUATIONS,
+      'feature_tag_value_1',
+      new ForbiddenException(),
+    )
+    interactor.trackFailure(
+      ApiId.REGISTER_EVENTS,
+      'feature_tag_value_1',
+      new InternalServerErrorException(),
+    )
+
+    const events = eventStorage
+      .getAll()
+      .map((e) => interactor.getMetricsEventUniqueKey(e.event as MetricsEvent))
+
+    expect(events).toStrictEqual([
+      '3::type.googleapis.com/bucketeer.event.client.InternalServerErrorMetricsEvent',
+    ])
+  })
+
   suite('sendEvents', () => {
     test('success', async () => {
       server.use(
         http.post<
           Record<string, never>,
           RegisterEventsRequest,
           RegisterEventsResponse
-          >(`${config.apiEndpoint}/register_events`, async ({ request }) => {
+          >(`${config.apiEndpoint}/register_events`, async ({request}) => {
           const body = await request.json()
           expect(body.events).toHaveLength(3)
           expect(body.sourceId).toEqual(SourceID.JAVASCRIPT)
@@ -452,7 +481,7 @@ suite('internal/event/EventInteractor', () => {
           Record<string, never>,
           RegisterEventsRequest,
           RegisterEventsResponse
-          >(`${config.apiEndpoint}/register_events`, async ({ request }) => {
+          >(`${config.apiEndpoint}/register_events`, async ({request}) => {
           const body = await request.json()
           expect(body.events).toHaveLength(3)
           return HttpResponse.json({

diff --git a/test/internal/remote/ApiClient.spec.ts b/test/internal/remote/ApiClient.spec.ts
@@ -209,17 +209,14 @@ suite('internal/remote/ApiClient', () => {
 
   suite('registerEvents', () => {
     test('success', async () => {
-      const requestInterceptor = vi.fn<
-        [StrictRequest<RegisterEventsRequest>],
-        void
-      >()
+      const requestInterceptor = vi.fn()
 
       server.use(
         http.post<
           Record<string, never>,
           RegisterEventsRequest,
           RegisterEventsResponse
-          >(`${endpoint}/register_events`, async ({ request }) => {
+          >(`${endpoint}/register_events`, async ({request}) => {
           requestInterceptor(request)
           return HttpResponse.json({
             errors: {
@@ -315,9 +312,10 @@ suite('internal/remote/ApiClient', () => {
         evaluationEvent1,
         metricsEvent1,
       ])
-
+
+      assert(response.type === 'failure')
       expect(response.type).toBe('failure')
-
+      
       const error = response.error
       expect(error.name).toBe('UnknownException')