fix: Async client calls were not captured by flush

HazAT · HazAT · commit 98d8260c7e8d · 2019-03-20T08:04:32.000+01:00
diff --git a/README.md b/README.md
@@ -45,7 +45,7 @@ package. Please refer to the README and instructions of those SDKs for more deta
 - [`@sentry/electron`](https://github.com/getsentry/sentry-electron): SDK for Electron with support for native crashes
 - [`sentry-cordova`](https://github.com/getsentry/sentry-cordova): SDK for Cordova Apps and Ionic with support for
   native crashes
-- [`raven-js`](https://github.com/getsentry/sentry-javascript/tree/master/packages/raven-js): Our old stable Javascript
+- [`raven-js`](https://github.com/getsentry/sentry-javascript/tree/master/packages/raven-js): Our old stable JavaScript
   SDK, we still support and release bug fixes for the SDK but all new features will be implemented in `@sentry/browser`
   which is the successor.
 - [`raven`](https://github.com/getsentry/sentry-javascript/tree/master/packages/raven-node): Our old stable Node SDK,
@@ -74,18 +74,6 @@ init({
 captureMessage('Hello, world!');
 ```
 
-If you want sentry to be customized for the browsers you want to support use the `esm` build:
-
-```javascript
-import { init, captureMessage } from '@sentry/browser/esm';
-```
-
-and add it to your babel/... build, if you want to support older browsers
-
-> TIP: You can use
-> [`<script type="module" src="newbrowser.js">` + `<script nomodule src="oldbrowser.js">`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#Module_Fallback)
-> to provide builds for newer and older browsers
-
 ## Other Packages
 
 Besides the high-level SDKs, this repository contains shared packages, helpers and configuration used for SDK
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -53,15 +53,15 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
   /** Options passed to the SDK. */
   protected readonly _options: O;
 
-  /**
-   * The client Dsn, if specified in options. Without this Dsn, the SDK will be
-   * disabled.
-   */
+  /** The client Dsn, if specified in options. Without this Dsn, the SDK will be disabled. */
   protected readonly _dsn?: Dsn;
 
   /** Array of used integrations. */
   protected readonly _integrations: IntegrationIndex;
 
+  /** Is the client still processing a call? */
+  protected _processing: boolean = false;
+
   /**
    * Initializes this client instance.
    *
@@ -84,15 +84,19 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
    */
   public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
     let eventId: string | undefined = hint && hint.event_id;
+    this._processing = true;
 
     this._getBackend()
       .eventFromException(exception, hint)
       .then(event => this._processEvent(event, hint, scope))
       .then(finalEvent => {
-        eventId = finalEvent.event_id;
+        // We need to check for finalEvent in case beforeSend returned null
+        eventId = finalEvent && finalEvent.event_id;
+        this._processing = false;
       })
       .catch(reason => {
         logger.log(reason);
+        this._processing = false;
       });
 
     return eventId;
@@ -104,17 +108,22 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
   public captureMessage(message: string, level?: Severity, hint?: EventHint, scope?: Scope): string | undefined {
     let eventId: string | undefined = hint && hint.event_id;
 
+    this._processing = true;
+
     const promisedEvent = isPrimitive(message)
       ? this._getBackend().eventFromMessage(`${message}`, level, hint)
       : this._getBackend().eventFromException(message, hint);
 
     promisedEvent
       .then(event => this._processEvent(event, hint, scope))
       .then(finalEvent => {
-        eventId = finalEvent.event_id;
+        // We need to check for finalEvent in case beforeSend returned null
+        eventId = finalEvent && finalEvent.event_id;
+        this._processing = false;
       })
       .catch(reason => {
         logger.log(reason);
+        this._processing = false;
       });
 
     return eventId;
@@ -125,12 +134,17 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
    */
   public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string | undefined {
     let eventId: string | undefined = hint && hint.event_id;
+    this._processing = true;
+
     this._processEvent(event, hint, scope)
       .then(finalEvent => {
-        eventId = finalEvent.event_id;
+        // We need to check for finalEvent in case beforeSend returned null
+        eventId = finalEvent && finalEvent.event_id;
+        this._processing = false;
       })
       .catch(reason => {
         logger.log(reason);
+        this._processing = false;
       });
     return eventId;
   }
@@ -149,6 +163,64 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     return this._options;
   }
 
+  /**
+   * @inheritDoc
+   */
+  public async flush(timeout?: number): Promise<boolean> {
+    return (await Promise.all([
+      this._getBackend()
+        .getTransport()
+        .close(timeout),
+      this._isClientProcessing(),
+    ])).reduce((prev, current) => prev && current);
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public async close(timeout?: number): Promise<boolean> {
+    return this.flush(timeout).finally(() => {
+      this.getOptions().enabled = false;
+    });
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public getIntegrations(): IntegrationIndex {
+    return this._integrations || {};
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
+    try {
+      return (this._integrations[integration.id] as T) || null;
+    } catch (_oO) {
+      logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
+      return null;
+    }
+  }
+
+  /** Waits for the client to be done with processing. */
+  protected async _isClientProcessing(counter: number = 0): Promise<boolean> {
+    return new Promise<boolean>(resolve => {
+      if (this._processing) {
+        // Safeguard in case of endless recursion
+        if (counter >= 10) {
+          resolve(false);
+        } else {
+          setTimeout(async () => {
+            resolve(await this._isClientProcessing(counter + 1));
+          }, 10);
+        }
+      } else {
+        resolve(true);
+      }
+    });
+  }
+
   /** Returns the current backend. */
   protected _getBackend(): B {
     return this._backend;
@@ -315,41 +387,4 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
         reject(`beforeSend rejected with ${e}`);
       });
   }
-
-  /**
-   * @inheritDoc
-   */
-  public async flush(timeout?: number): Promise<boolean> {
-    return this._getBackend()
-      .getTransport()
-      .close(timeout);
-  }
-
-  /**
-   * @inheritDoc
-   */
-  public async close(timeout?: number): Promise<boolean> {
-    return this.flush(timeout).finally(() => {
-      this.getOptions().enabled = false;
-    });
-  }
-
-  /**
-   * @inheritDoc
-   */
-  public getIntegrations(): IntegrationIndex {
-    return this._integrations || {};
-  }
-
-  /**
-   * @inheritDoc
-   */
-  public getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
-    try {
-      return (this._integrations[integration.id] as T) || null;
-    } catch (_oO) {
-      logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
-      return null;
-    }
-  }
 }
diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts
@@ -81,6 +81,7 @@ export class NodeBackend extends BaseBackend<NodeOptions> {
       handled: true,
       type: 'generic',
     };
+
     if (!isError(exception)) {
       if (isPlainObject(exception)) {
         // This will allow us to group events based on top-level keys
@@ -102,14 +103,16 @@ export class NodeBackend extends BaseBackend<NodeOptions> {
       mechanism.synthetic = true;
     }
 
-    return new SyncPromise<Event>(resolve =>
-      parseError(ex as Error, this._options).then(event => {
-        addExceptionTypeValue(event, undefined, undefined, mechanism);
-        resolve({
-          ...event,
-          event_id: hint && hint.event_id,
-        });
-      }),
+    return new SyncPromise<Event>((resolve, reject) =>
+      parseError(ex as Error, this._options)
+        .then(event => {
+          addExceptionTypeValue(event, undefined, undefined, mechanism);
+          resolve({
+            ...event,
+            event_id: hint && hint.event_id,
+          });
+        })
+        .catch(reject),
     );
   }
 
