feat: Add queue to backends and client + reason of skip

Author: HazAT

packages/browser/src/backend.ts

@@ -1,4 +1,4 @@
-import { Backend, logger, Options, SentryError } from '@sentry/core';
+import { Backend, logger, Options, Queue, SentryError } from '@sentry/core';
 import { SentryEvent, SentryEventHint, SentryResponse, Severity, Status, Transport } from '@sentry/types';
 import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is';
 import { supportsBeacon, supportsFetch } from '@sentry/utils/supports';
@@ -40,6 +40,9 @@ export class BrowserBackend implements Backend {
   /** Cached transport used internally. */
   private transport?: Transport;
 
+  /** A simple queue holding all requests. */
+  private queue: Queue<SentryResponse> = new Queue();
+
   /**
    * @inheritDoc
    */
@@ -141,8 +144,8 @@ export class BrowserBackend implements Backend {
   public async sendEvent(event: SentryEvent): Promise<SentryResponse> {
     if (!this.options.dsn) {
       logger.warn(`Event has been skipped because no Dsn is configured.`);
-      // We do nothing in case there is no Dsn
-      return { status: Status.Skipped };
+      // We do nothing in case there is no DSN
+      return { status: Status.Skipped, reason: `Event has been skipped because no Dsn is configured.` };
     }
 
     if (!this.transport) {
@@ -161,7 +164,7 @@ export class BrowserBackend implements Backend {
       }
     }
 
-    return this.transport.captureEvent(event);
+    return this.queue.add(this.transport.captureEvent(event));
   }
 
   /**
@@ -177,4 +180,11 @@ export class BrowserBackend implements Backend {
   public storeScope(): void {
     // Noop
   }
+
+  /**
+   * @inheritDoc
+   */
+  public close(timeout?: number): Promise<boolean> {
+    return this.queue.drain(timeout);
+  }
 }

packages/core/src/base.ts

@@ -310,4 +310,11 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
 
     return response;
   }
+
+  /**
+   * @inheritDoc
+   */
+  public close(timeout?: number): Promise<boolean> {
+    return this.backend.close(timeout);
+  }
 }

packages/core/src/index.ts

@@ -5,5 +5,6 @@ export { API } from './api';
 export { BackendClass, BaseClient } from './base';
 export { Dsn } from './dsn';
 export { SentryError } from './error';
+export { Queue } from './queue';
 export { Backend, Client, LogLevel, Options } from './interfaces';
 export { initAndBind, ClientClass } from './sdk';

packages/core/src/interfaces.ts

@@ -194,6 +194,15 @@ export interface Client<O extends Options = Options> {
 
   /** Returns the current options. */
   getOptions(): O;
+
+  /**
+   * A promise that resolves whenever the queue is empty.
+   * If you provide a timeout and the queue takes longer to drain the promise returns false.
+   * Calls {@link Backend.close}.
+   *
+   * @param timeout Maximum time in ms the client should wait.
+   */
+  close(timeout?: number): Promise<boolean>;
 }
 
 /**
@@ -254,4 +263,12 @@ export interface Backend {
    * @param scope The scope to store.
    */
   storeScope(scope: Scope): void;
+
+  /**
+   * A promise that resolves whenever the queue is empty.
+   * If you provide a timeout and the queue takes longer to drain the promise returns false.
+   *
+   * @param timeout Maximum time in ms the client should wait.
+   */
+  close(timeout?: number): Promise<boolean>;
 }

packages/core/src/queue.ts

@@ -3,25 +3,29 @@ export class Queue<T> {
 
   public add(task: Promise<T>): Promise<T> {
     this.queue.add(task);
-    task.then(() => this.queue.delete(task));
+    task.then(() => this.queue.delete(task)).catch(() => this.queue.delete(task));
     return task;
   }
 
   public length(): number {
     return this.queue.size;
   }
 
-  public drain(timeout?: number): Promise<void> {
-    return new Promise((resolve, reject) => {
+  public drain(timeout?: number): Promise<boolean> {
+    return new Promise(resolve => {
       const capturedSetTimeout = setTimeout(() => {
         if (timeout && timeout > 0) {
-          reject('Drain timeout reached');
+          resolve(false);
         }
       }, timeout);
-      Promise.all(this.queue.values()).then(() => {
-        clearTimeout(capturedSetTimeout);
-        resolve();
-      });
+      Promise.all(this.queue.values())
+        .then(() => {
+          clearTimeout(capturedSetTimeout);
+          resolve(true);
+        })
+        .catch(() => {
+          resolve(true);
+        });
     });
   }
 }

packages/core/test/lib/queue.test.ts

@@ -43,14 +43,15 @@ describe('Queue', () => {
   });
 
   test('drain()', async () => {
-    expect.assertions(2);
+    expect.assertions(3);
     const q = new Queue<void>();
     for (let i = 0; i < 5; i++) {
       const p = new Promise<void>(resolve => setTimeout(resolve, 1));
       q.add(p);
     }
     expect(q.length()).toBe(5);
-    q.drain().then(() => {
+    q.drain().then(result => {
+      expect(result).toBeTruthy();
       expect(q.length()).toBe(0);
     });
     jest.runAllTimers();
@@ -64,8 +65,8 @@ describe('Queue', () => {
       q.add(p);
     }
     expect(q.length()).toBe(5);
-    q.drain(50).catch(e => {
-      expect(e).toBeTruthy();
+    q.drain(50).then(result => {
+      expect(result).toBeFalsy();
     });
     jest.runAllTimers();
   });

packages/node/src/backend.ts

@@ -1,4 +1,4 @@
-import { Backend, Dsn, Options, SentryError } from '@sentry/core';
+import { Backend, Dsn, Options, Queue, SentryError } from '@sentry/core';
 import { getCurrentHub } from '@sentry/hub';
 import { SentryEvent, SentryEventHint, SentryResponse, Severity, Transport } from '@sentry/types';
 import { isError, isPlainObject } from '@sentry/utils/is';
@@ -27,6 +27,9 @@ export class NodeBackend implements Backend {
   /** Cached transport used internally. */
   private transport?: Transport;
 
+  /** A simple queue holding all requests. */
+  private queue: Queue<SentryResponse> = new Queue();
+
   /**
    * @inheritDoc
    */
@@ -105,7 +108,7 @@ export class NodeBackend implements Backend {
           : new HTTPSTransport(transportOptions);
     }
 
-    return this.transport.captureEvent(event);
+    return this.queue.add(this.transport.captureEvent(event));
   }
 
   /**
@@ -121,4 +124,11 @@ export class NodeBackend implements Backend {
   public storeScope(): void {
     // Noop
   }
+
+  /**
+   * @inheritDoc
+   */
+  public close(timeout?: number): Promise<boolean> {
+    return this.queue.drain(timeout);
+  }
 }

packages/node/test/index.test.ts

@@ -13,12 +13,26 @@ import {
   Scope,
   SentryEvent,
   SentryResponse,
+  Status,
 } from '../src';
+import { BaseTransport } from '../src/transports';
 
 const dsn = 'https://[email protected]/4291';
 
 declare var global: any;
 
+class SetTimeoutTransport extends BaseTransport {
+  public async captureEvent(_: SentryEvent): Promise<SentryResponse> {
+    return new Promise<SentryResponse>(resolve => {
+      setTimeout(() => {
+        resolve({
+          status: Status.fromHttpCode(200),
+        });
+      }, 1);
+    });
+  }
+}
+
 describe('SentryNode', () => {
   beforeAll(() => {
     init({ dsn });
@@ -32,6 +46,47 @@ describe('SentryNode', () => {
     getCurrentHub().popScope();
   });
 
+  test('close() with to short timeout', done => {
+    expect.assertions(1);
+    jest.useFakeTimers();
+    getCurrentHub().pushScope();
+    const client = new NodeClient({
+      dsn,
+      transport: SetTimeoutTransport,
+    });
+    getCurrentHub().bindClient(client);
+    captureMessage('test');
+    captureMessage('test');
+    captureMessage('test');
+    client.close(50).then(result => {
+      expect(result).toBeFalsy();
+      done();
+    });
+    jest.runAllTimers();
+    getCurrentHub().popScope();
+  });
+
+  test('close() with timeout', done => {
+    expect.assertions(1);
+    jest.useFakeTimers();
+    getCurrentHub().pushScope();
+    const client = new NodeClient({
+      dsn,
+      transport: SetTimeoutTransport,
+    });
+    getCurrentHub().bindClient(client);
+    captureMessage('test');
+    captureMessage('test');
+    captureMessage('test');
+    jest.runAllTimers();
+    client.close(50).then(result => {
+      expect(result).toBeFalsy();
+      done();
+    });
+    jest.runAllTimers();
+    getCurrentHub().popScope();
+  });
+
   describe('getContext() / setContext()', () => {
     test('store/load extra', async () => {
       configureScope((scope: Scope) => {
@@ -111,10 +166,10 @@ describe('SentryNode', () => {
           beforeSend: (event: SentryEvent) => {
             expect(event.tags).toEqual({ test: '1' });
             expect(event.exception).not.toBeUndefined();
-            expect(event.exception!.values[0]).not.toBeUndefined();
-            expect(event.exception!.values[0].type).toBe('Error');
-            expect(event.exception!.values[0].value).toBe('test');
-            expect(event.exception!.values[0].stacktrace).toBeTruthy();
+            expect(event.exception!.values![0]).not.toBeUndefined();
+            expect(event.exception!.values![0].type).toBe('Error');
+            expect(event.exception!.values![0].value).toBe('test');
+            expect(event.exception!.values![0].stacktrace).toBeTruthy();
             done();
             return event;
           },

packages/types/src/index.ts

@@ -211,6 +211,7 @@ export interface Integration {
 export interface SentryResponse {
   status: Status;
   event?: SentryEvent;
+  reason?: string;
 }
 
 /** JSDoc */