Unit tests for recorder and recording

kamilogorek · kamilogorek · commit 2de0e3fcc24b · 2021-09-15T12:17:42.000+02:00
diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts
@@ -42,7 +42,7 @@ export abstract class BaseTransport implements Transport {
     // eslint-disable-next-line deprecation/deprecation
     this.url = this._api.getStoreEndpointWithUrlEncodedAuth();
 
-    if (this.options.sendClientReport) {
+    if (this.options.sendClientReports) {
       document.addEventListener('visibilitychange', () => {
         if (document.visibilityState === 'hidden') {
           this._flushOutcomes();
@@ -69,15 +69,15 @@ export abstract class BaseTransport implements Transport {
    * @inheritDoc
    */
   public recordLostEvent(reason: Outcome, category: SentryRequestType): void {
-    if (!this.options.sendClientReport) {
+    if (!this.options.sendClientReports) {
       return;
     }
     // We want to track each category (event, transaction, session) separately
     // but still keep the distinction between different type of outcomes.
     // We could use nested maps, but it's much easier to read and type this way.
     // A correct type for map-based implementation if we want to go that route
     // would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
-    const key = `${CATEGORY_MAPPING[category]}:${reason}}`;
+    const key = `${CATEGORY_MAPPING[category]}:${reason}`;
     logger.log(`Adding outcome: ${key}`);
     this._outcomes[key] = (this._outcomes[key] ?? 0) + 1;
   }
@@ -86,7 +86,7 @@ export abstract class BaseTransport implements Transport {
    * Send outcomes as an envelope
    */
   protected _flushOutcomes(): void {
-    if (!this.options.sendClientReport) {
+    if (!this.options.sendClientReports) {
       return;
     }
 
diff --git a/packages/browser/test/unit/transports/base.test.ts b/packages/browser/test/unit/transports/base.test.ts
@@ -1,10 +1,99 @@
+import { Outcome } from '@sentry/types';
+
 import { BaseTransport } from '../../../src/transports/base';
 
 const testDsn = 'https://123@sentry.io/42';
+const envelopeEndpoint = 'https://sentry.io/api/42/envelope/?sentry_key=123&sentry_version=7';
 
 class SimpleTransport extends BaseTransport {}
 
 describe('BaseTransport', () => {
+  describe('Client Reports', () => {
+    const sendBeaconSpy = jest.fn();
+    let visibilityState: string;
+
+    beforeAll(() => {
+      navigator.sendBeacon = sendBeaconSpy;
+      Object.defineProperty(document, 'visibilityState', {
+        configurable: true,
+        get: function() {
+          return visibilityState;
+        },
+      });
+      jest.spyOn(Date, 'now').mockImplementation(() => 1337);
+    });
+
+    beforeEach(() => {
+      sendBeaconSpy.mockClear();
+    });
+
+    it('attaches visibilitychange handler if sendClientReport is set to true', () => {
+      const eventListenerSpy = jest.spyOn(document, 'addEventListener');
+      new SimpleTransport({ dsn: testDsn, sendClientReports: true });
+      expect(eventListenerSpy.mock.calls[0][0]).toBe('visibilitychange');
+      eventListenerSpy.mockRestore();
+    });
+
+    it('doesnt attach visibilitychange handler if sendClientReport is set to false', () => {
+      const eventListenerSpy = jest.spyOn(document, 'addEventListener');
+      new SimpleTransport({ dsn: testDsn, sendClientReports: false });
+      expect(eventListenerSpy).not.toHaveBeenCalled();
+      eventListenerSpy.mockRestore();
+    });
+
+    it('sends beacon request when there are outcomes captured and visibility changed to `hidden`', () => {
+      const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });
+
+      transport.recordLostEvent(Outcome.BeforeSend, 'event');
+
+      visibilityState = 'hidden';
+      document.dispatchEvent(new Event('visibilitychange'));
+
+      const outcomes = [{ reason: 'before_send', category: 'error', quantity: 1 }];
+
+      expect(sendBeaconSpy).toHaveBeenCalledWith(
+        envelopeEndpoint,
+        `{"type":"client_report"}\n{"timestamp":1337,"discarded_events":${JSON.stringify(outcomes)}}`,
+      );
+    });
+
+    it('doesnt send beacon request when there are outcomes captured, but visibility state did not change to `hidden`', () => {
+      const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });
+      transport.recordLostEvent(Outcome.BeforeSend, 'event');
+
+      visibilityState = 'visible';
+      document.dispatchEvent(new Event('visibilitychange'));
+
+      expect(sendBeaconSpy).not.toHaveBeenCalled();
+    });
+
+    it('correctly serializes request with different categories/reasons pairs', () => {
+      const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });
+
+      transport.recordLostEvent(Outcome.BeforeSend, 'event');
+      transport.recordLostEvent(Outcome.BeforeSend, 'event');
+      transport.recordLostEvent(Outcome.SampleRate, 'transaction');
+      transport.recordLostEvent(Outcome.NetworkError, 'session');
+      transport.recordLostEvent(Outcome.NetworkError, 'session');
+      transport.recordLostEvent(Outcome.RateLimit, 'event');
+
+      visibilityState = 'hidden';
+      document.dispatchEvent(new Event('visibilitychange'));
+
+      const outcomes = [
+        { reason: 'before_send', category: 'error', quantity: 2 },
+        { reason: 'sample_rate', category: 'transaction', quantity: 1 },
+        { reason: 'network_error', category: 'session', quantity: 2 },
+        { reason: 'rate_limit', category: 'error', quantity: 1 },
+      ];
+
+      expect(sendBeaconSpy).toHaveBeenCalledWith(
+        envelopeEndpoint,
+        `{"type":"client_report"}\n{"timestamp":1337,"discarded_events":${JSON.stringify(outcomes)}}`,
+      );
+    });
+  });
+
   it('doesnt provide sendEvent() implementation', () => {
     const transport = new SimpleTransport({ dsn: testDsn });
 
diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts
@@ -1,3 +1,6 @@
+import { Outcome } from '@sentry/types';
+import { SentryError } from '@sentry/utils';
+
 import { Event, Response, Status, Transports } from '../../../src';
 
 const testDsn = 'https://123@sentry.io/42';
@@ -104,6 +107,32 @@ describe('FetchTransport', () => {
       }
     });
 
+    it('should record dropped event when fetch fails', async () => {
+      const response = { status: 403, headers: new Headers() };
+
+      window.fetch.mockImplementation(() => Promise.reject(response));
+
+      const spy = jest.spyOn(transport, 'recordLostEvent');
+
+      try {
+        await transport.sendEvent(eventPayload);
+      } catch (_) {
+        expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event');
+      }
+    });
+
+    it('should record dropped event when queue buffer overflows', async () => {
+      // @ts-ignore private method
+      jest.spyOn(transport._buffer, 'add').mockRejectedValue(new SentryError('Buffer Full'));
+      const spy = jest.spyOn(transport, 'recordLostEvent');
+
+      try {
+        await transport.sendEvent(transactionPayload);
+      } catch (_) {
+        expect(spy).toHaveBeenCalledWith(Outcome.QueueSize, 'transaction');
+      }
+    });
+
     it('passes in headers', async () => {
       transport = new Transports.FetchTransport(
         {
@@ -451,6 +480,25 @@ describe('FetchTransport', () => {
         expect(eventRes.status).toBe(Status.Success);
         expect(fetch).toHaveBeenCalledTimes(2);
       });
+
+      it('should record dropped event', async () => {
+        // @ts-ignore private method
+        jest.spyOn(transport, '_isRateLimited').mockReturnValue(true);
+
+        const spy = jest.spyOn(transport, 'recordLostEvent');
+
+        try {
+          await transport.sendEvent(eventPayload);
+        } catch (_) {
+          expect(spy).toHaveBeenCalledWith(Outcome.RateLimit, 'event');
+        }
+
+        try {
+          await transport.sendEvent(transactionPayload);
+        } catch (_) {
+          expect(spy).toHaveBeenCalledWith(Outcome.RateLimit, 'transaction');
+        }
+      });
     });
   });
 });
diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts
@@ -1,3 +1,5 @@
+import { Outcome } from '@sentry/types';
+import { SentryError } from '@sentry/utils';
 import { fakeServer, SinonFakeServer } from 'sinon';
 
 import { Event, Response, Status, Transports } from '../../../src';
@@ -69,6 +71,30 @@ describe('XHRTransport', () => {
       }
     });
 
+    it('should record dropped event when request fails', async () => {
+      server.respondWith('POST', storeUrl, [403, {}, '']);
+
+      const spy = jest.spyOn(transport, 'recordLostEvent');
+
+      try {
+        await transport.sendEvent(eventPayload);
+      } catch (_) {
+        expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event');
+      }
+    });
+
+    it('should record dropped event when queue buffer overflows', async () => {
+      // @ts-ignore private method
+      jest.spyOn(transport._buffer, 'add').mockRejectedValue(new SentryError('Buffer Full'));
+      const spy = jest.spyOn(transport, 'recordLostEvent');
+
+      try {
+        await transport.sendEvent(transactionPayload);
+      } catch (_) {
+        expect(spy).toHaveBeenCalledWith(Outcome.QueueSize, 'transaction');
+      }
+    });
+
     it('passes in headers', async () => {
       transport = new Transports.XHRTransport({
         dsn: testDsn,
@@ -380,6 +406,25 @@ describe('XHRTransport', () => {
         expect(eventRes.status).toBe(Status.Success);
         expect(server.requests.length).toBe(2);
       });
+
+      it('should record dropped event', async () => {
+        // @ts-ignore private method
+        jest.spyOn(transport, '_isRateLimited').mockReturnValue(true);
+
+        const spy = jest.spyOn(transport, 'recordLostEvent');
+
+        try {
+          await transport.sendEvent(eventPayload);
+        } catch (_) {
+          expect(spy).toHaveBeenCalledWith(Outcome.RateLimit, 'event');
+        }
+
+        try {
+          await transport.sendEvent(transactionPayload);
+        } catch (_) {
+          expect(spy).toHaveBeenCalledWith(Outcome.RateLimit, 'transaction');
+        }
+      });
     });
   });
 });
diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts
@@ -1,5 +1,5 @@
 import { Hub, Scope, Session } from '@sentry/hub';
-import { Event, Severity, Span } from '@sentry/types';
+import { Event, Outcome, Severity, Span, Transport } from '@sentry/types';
 import { logger, SentryError, SyncPromise } from '@sentry/utils';
 
 import * as integrationModule from '../../src/integration';
@@ -807,6 +807,28 @@ describe('BaseClient', () => {
       expect((TestBackend.instance!.event! as any).data).toBe('someRandomThing');
     });
 
+    test('beforeSend records dropped events', () => {
+      expect.assertions(1);
+      const client = new TestClient({
+        dsn: PUBLIC_DSN,
+        beforeSend() {
+          return null;
+        },
+      });
+
+      const recordLostEventSpy = jest.fn();
+      jest.spyOn(client, 'getTransport').mockImplementationOnce(
+        () =>
+          (({
+            recordLostEvent: recordLostEventSpy,
+          } as any) as Transport),
+      );
+
+      client.captureEvent({ message: 'hello' }, {});
+
+      expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.BeforeSend, 'event');
+    });
+
     test('eventProcessor can drop the even when it returns null', () => {
       expect.assertions(3);
       const client = new TestClient({ dsn: PUBLIC_DSN });
@@ -820,6 +842,25 @@ describe('BaseClient', () => {
       expect(loggerErrorSpy).toBeCalledWith(new SentryError('An event processor returned null, will not send event.'));
     });
 
+    test('eventProcessor records dropped events', () => {
+      expect.assertions(1);
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+
+      const recordLostEventSpy = jest.fn();
+      jest.spyOn(client, 'getTransport').mockImplementationOnce(
+        () =>
+          (({
+            recordLostEvent: recordLostEventSpy,
+          } as any) as Transport),
+      );
+
+      const scope = new Scope();
+      scope.addEventProcessor(() => null);
+      client.captureEvent({ message: 'hello' }, {}, scope);
+
+      expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.EventProcessor, 'event');
+    });
+
     test('eventProcessor sends an event and logs when it crashes', () => {
       expect.assertions(3);
       const client = new TestClient({ dsn: PUBLIC_DSN });
@@ -844,6 +885,25 @@ describe('BaseClient', () => {
         ),
       );
     });
+
+    test('records events dropped due to sampleRate', () => {
+      expect.assertions(1);
+      const client = new TestClient({
+        dsn: PUBLIC_DSN,
+        sampleRate: 0,
+      });
+
+      const recordLostEventSpy = jest.fn();
+      jest.spyOn(client, 'getTransport').mockImplementationOnce(
+        () =>
+          (({
+            recordLostEvent: recordLostEventSpy,
+          } as any) as Transport),
+      );
+
+      client.captureEvent({ message: 'hello' }, {});
+      expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.SampleRate, 'event');
+    });
   });
 
   describe('integrations', () => {
diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts
@@ -1,13 +1,17 @@
-import { BrowserClient } from '@sentry/browser';
+import { BrowserClient, Transports } from '@sentry/browser';
 import { Hub } from '@sentry/hub';
+import { Outcome } from '@sentry/types';
 
 import { DEFAULT_IDLE_TIMEOUT, IdleTransaction, IdleTransactionSpanRecorder } from '../src/idletransaction';
 import { Span } from '../src/span';
 import { SpanStatus } from '../src/spanstatus';
 
+export class SimpleTransport extends Transports.BaseTransport {}
+
+const dsn = 'https://123@sentry.io/42';
 let hub: Hub;
 beforeEach(() => {
-  hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
+  hub = new Hub(new BrowserClient({ dsn, tracesSampleRate: 1, transport: SimpleTransport }));
 });
 
 describe('IdleTransaction', () => {
@@ -156,6 +160,19 @@ describe('IdleTransaction', () => {
     }
   });
 
+  it('should record dropped transactions', async () => {
+    const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234, sampled: false }, hub, 1000);
+
+    const transport = hub.getClient()?.getTransport();
+
+    const spy = jest.spyOn(transport!, 'recordLostEvent');
+
+    transaction.initSpanRecorder(10);
+    transaction.finish(transaction.startTimestamp + 10);
+
+    expect(spy).toHaveBeenCalledWith(Outcome.SampleRate, 'transaction');
+  });
+
   describe('_initTimeout', () => {
     it('finishes if no activities are added to the transaction', () => {
       const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, 1000);
diff --git a/packages/types/src/request.ts b/packages/types/src/request.ts
@@ -1,4 +1,5 @@
 /** Possible SentryRequest types that can be used to make a distinction between Sentry features */
+// NOTE(kamil): It would be nice if we make it a valid enum instead
 export type SentryRequestType = 'event' | 'transaction' | 'session' | 'attachment';
 
 /** A generic client request. */
diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`/** Possible SentryRequest types that can be used to make a distinction between Sentry features */`
	`2`	`+// NOTE(kamil): It would be nice if we make it a valid enum instead`
`2`	`3`	`export type SentryRequestType = 'event' \| 'transaction' \| 'session' \| 'attachment';`
`3`	`4`
`4`	`5`	`/** A generic client request. */`