Merge branch 'master' into apm-performance-api

getsentry · Mar 9, 2020 · 3b34841 · 3b34841
2 parents eb55285 + dd7bf92
commit 3b34841
Show file tree

Hide file tree

Showing 4 changed files with 155 additions and 103 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
 - [apm] feat: Make use of the `performance` browser API to provide better instrumentation (#2474)
+- [browser] ref: Move global error handler + unhandled promise rejection to instrument
 
 ## 5.13.2
 

diff --git a/packages/apm/src/integrations/tracing.ts b/packages/apm/src/integrations/tracing.ts
@@ -216,6 +216,27 @@ export class Tracing implements Integration {
       });
     }
 
+    /**
+     * If an error or unhandled promise occurs, we mark the active transaction as failed
+     */
+    // tslint:disable-next-line: completed-docs
+    function errorCallback(): void {
+      if (Tracing._activeTransaction) {
+        logger.log(`[Tracing] Global error occured, setting status in transaction: ${SpanStatus.InternalError}`);
+        (Tracing._activeTransaction as SpanClass).setStatus(SpanStatus.InternalError);
+      }
+    }
+
+    addInstrumentationHandler({
+      callback: errorCallback,
+      type: 'error',
+    });
+
+    addInstrumentationHandler({
+      callback: errorCallback,
+      type: 'unhandledrejection',
+    });
+
     if (Tracing.options.discardBackgroundSpans && global.document) {
       document.addEventListener('visibilitychange', () => {
         if (document.hidden && Tracing._activeTransaction) {

diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts
@@ -2,7 +2,7 @@ import { getCurrentHub } from '@sentry/core';
 import { Event, Integration, Severity } from '@sentry/types';
 import {
   addExceptionMechanism,
-  getGlobalObject,
+  addInstrumentationHandler,
   getLocationHref,
   isErrorEvent,
   isPrimitive,
@@ -34,15 +34,6 @@ export class GlobalHandlers implements Integration {
   /** JSDoc */
   private readonly _options: GlobalHandlersIntegrations;
 
-  /** JSDoc */
-  private readonly _global: Window = getGlobalObject();
-
-  /** JSDoc */
-  private _oldOnErrorHandler: OnErrorEventHandler = null;
-
-  /** JSDoc */
-  private _oldOnUnhandledRejectionHandler: ((e: any) => void) | null = null;
-
   /** JSDoc */
   private _onErrorHandlerInstalled: boolean = false;
 
@@ -80,49 +71,41 @@ export class GlobalHandlers implements Integration {
       return;
     }
 
-    const self = this; // tslint:disable-line:no-this-assignment
-    this._oldOnErrorHandler = this._global.onerror;
+    addInstrumentationHandler({
+      callback: (data: { msg: any; url: any; line: any; column: any; error: any }) => {
+        const error = data.error;
+        const currentHub = getCurrentHub();
+        const hasIntegration = currentHub.getIntegration(GlobalHandlers);
+        const isFailedOwnDelivery = error && error.__sentry_own_request__ === true;
 
-    this._global.onerror = function(msg: any, url: any, line: any, column: any, error: any): boolean {
-      const currentHub = getCurrentHub();
-      const hasIntegration = currentHub.getIntegration(GlobalHandlers);
-      const isFailedOwnDelivery = error && error.__sentry_own_request__ === true;
-
-      if (!hasIntegration || shouldIgnoreOnError() || isFailedOwnDelivery) {
-        if (self._oldOnErrorHandler) {
-          return self._oldOnErrorHandler.apply(this, arguments);
+        if (!hasIntegration || shouldIgnoreOnError() || isFailedOwnDelivery) {
+          return;
         }
-        return false;
-      }
-
-      const client = currentHub.getClient();
-      const event = isPrimitive(error)
-        ? self._eventFromIncompleteOnError(msg, url, line, column)
-        : self._enhanceEventWithInitialFrame(
-            eventFromUnknownInput(error, undefined, {
-              attachStacktrace: client && client.getOptions().attachStacktrace,
-              rejection: false,
-            }),
-            url,
-            line,
-            column,
-          );
-
-      addExceptionMechanism(event, {
-        handled: false,
-        type: 'onerror',
-      });
-
-      currentHub.captureEvent(event, {
-        originalException: error,
-      });
-
-      if (self._oldOnErrorHandler) {
-        return self._oldOnErrorHandler.apply(this, arguments);
-      }
 
-      return false;
-    };
+        const client = currentHub.getClient();
+        const event = isPrimitive(error)
+          ? this._eventFromIncompleteOnError(data.msg, data.url, data.line, data.column)
+          : this._enhanceEventWithInitialFrame(
+              eventFromUnknownInput(error, undefined, {
+                attachStacktrace: client && client.getOptions().attachStacktrace,
+                rejection: false,
+              }),
+              data.url,
+              data.line,
+              data.column,
+            );
+
+        addExceptionMechanism(event, {
+          handled: false,
+          type: 'onerror',
+        });
+
+        currentHub.captureEvent(event, {
+          originalException: error,
+        });
+      },
+      type: 'error',
+    });
 
     this._onErrorHandlerInstalled = true;
   }
@@ -133,67 +116,60 @@ export class GlobalHandlers implements Integration {
       return;
     }
 
-    const self = this; // tslint:disable-line:no-this-assignment
-    this._oldOnUnhandledRejectionHandler = this._global.onunhandledrejection;
-
-    this._global.onunhandledrejection = function(e: any): boolean {
-      let error = e;
-
-      // dig the object of the rejection out of known event types
-      try {
-        // PromiseRejectionEvents store the object of the rejection under 'reason'
-        // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent
-        if ('reason' in e) {
-          error = e.reason;
+    addInstrumentationHandler({
+      callback: (e: any) => {
+        let error = e;
+
+        // dig the object of the rejection out of known event types
+        try {
+          // PromiseRejectionEvents store the object of the rejection under 'reason'
+          // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent
+          if ('reason' in e) {
+            error = e.reason;
+          }
+          // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents
+          // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into
+          // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec
+          // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and
+          // https://github.com/getsentry/sentry-javascript/issues/2380
+          else if ('detail' in e && 'reason' in e.detail) {
+            error = e.detail.reason;
+          }
+        } catch (_oO) {
+          // no-empty
         }
-        // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents
-        // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into
-        // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec
-        // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and
-        // https://github.com/getsentry/sentry-javascript/issues/2380
-        else if ('detail' in e && 'reason' in e.detail) {
-          error = e.detail.reason;
-        }
-      } catch (_oO) {
-        // no-empty
-      }
 
-      const currentHub = getCurrentHub();
-      const hasIntegration = currentHub.getIntegration(GlobalHandlers);
-      const isFailedOwnDelivery = error && error.__sentry_own_request__ === true;
+        const currentHub = getCurrentHub();
+        const hasIntegration = currentHub.getIntegration(GlobalHandlers);
+        const isFailedOwnDelivery = error && error.__sentry_own_request__ === true;
 
-      if (!hasIntegration || shouldIgnoreOnError() || isFailedOwnDelivery) {
-        if (self._oldOnUnhandledRejectionHandler) {
-          return self._oldOnUnhandledRejectionHandler.apply(this, arguments);
+        if (!hasIntegration || shouldIgnoreOnError() || isFailedOwnDelivery) {
+          return true;
         }
-        return true;
-      }
 
-      const client = currentHub.getClient();
-      const event = isPrimitive(error)
-        ? self._eventFromIncompleteRejection(error)
-        : eventFromUnknownInput(error, undefined, {
-            attachStacktrace: client && client.getOptions().attachStacktrace,
-            rejection: true,
-          });
+        const client = currentHub.getClient();
+        const event = isPrimitive(error)
+          ? this._eventFromIncompleteRejection(error)
+          : eventFromUnknownInput(error, undefined, {
+              attachStacktrace: client && client.getOptions().attachStacktrace,
+              rejection: true,
+            });
 
-      event.level = Severity.Error;
+        event.level = Severity.Error;
 
-      addExceptionMechanism(event, {
-        handled: false,
-        type: 'onunhandledrejection',
-      });
+        addExceptionMechanism(event, {
+          handled: false,
+          type: 'onunhandledrejection',
+        });
 
-      currentHub.captureEvent(event, {
-        originalException: error,
-      });
+        currentHub.captureEvent(event, {
+          originalException: error,
+        });
 
-      if (self._oldOnUnhandledRejectionHandler) {
-        return self._oldOnUnhandledRejectionHandler.apply(this, arguments);
-      }
-
-      return true;
-    };
+        return;
+      },
+      type: 'unhandledrejection',
+    });
 
     this._onUnhandledRejectionHandlerInstalled = true;
   }

diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts
@@ -15,7 +15,15 @@ interface InstrumentHandler {
   type: InstrumentHandlerType;
   callback: InstrumentHandlerCallback;
 }
-type InstrumentHandlerType = 'console' | 'dom' | 'fetch' | 'history' | 'sentry' | 'xhr';
+type InstrumentHandlerType =
+  | 'console'
+  | 'dom'
+  | 'fetch'
+  | 'history'
+  | 'sentry'
+  | 'xhr'
+  | 'error'
+  | 'unhandledrejection';
 type InstrumentHandlerCallback = (data: any) => void;
 
 /**
@@ -25,6 +33,8 @@ type InstrumentHandlerCallback = (data: any) => void;
  *  - XHR API
  *  - History API
  *  - DOM API (click/typing)
+ *  - Error API
+ *  - UnhandledRejection API
  */
 
 const handlers: { [key in InstrumentHandlerType]?: InstrumentHandlerCallback[] } = {};
@@ -54,6 +64,12 @@ function instrument(type: InstrumentHandlerType): void {
     case 'history':
       instrumentHistory();
       break;
+    case 'error':
+      instrumentError();
+      break;
+    case 'unhandledrejection':
+      instrumentUnhandledRejection();
+      break;
     default:
       logger.warn('unknown instrumentation type:', type);
   }
@@ -471,3 +487,41 @@ function keypressEventHandler(handler: Function): (event: Event) => void {
     }, debounceDuration) as any) as number;
   };
 }
+
+let _oldOnErrorHandler: OnErrorEventHandler = null;
+/** JSDoc */
+function instrumentError(): void {
+  _oldOnErrorHandler = global.onerror;
+
+  global.onerror = function(msg: any, url: any, line: any, column: any, error: any): boolean {
+    triggerHandlers('error', {
+      column,
+      error,
+      line,
+      msg,
+      url,
+    });
+
+    if (_oldOnErrorHandler) {
+      return _oldOnErrorHandler.apply(this, arguments);
+    }
+
+    return false;
+  };
+}
+
+let _oldOnUnhandledRejectionHandler: ((e: any) => void) | null = null;
+/** JSDoc */
+function instrumentUnhandledRejection(): void {
+  _oldOnUnhandledRejectionHandler = global.onunhandledrejection;
+
+  global.onunhandledrejection = function(e: any): boolean {
+    triggerHandlers('unhandledrejection', e);
+
+    if (_oldOnUnhandledRejectionHandler) {
+      return _oldOnUnhandledRejectionHandler.apply(this, arguments);
+    }
+
+    return true;
+  };
+}