ref(tracing): Make sure error status is set on transactions (#2818)

* ref: Make sure error status is set on transactions

Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>

Author: AbhiPrasad

packages/tracing/src/browser/browsertracing.ts

@@ -7,7 +7,6 @@ import { DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../idletransaction';
 import { Span } from '../span';
 import { SpanStatus } from '../spanstatus';
 import { registerBackgroundTabDetection } from './backgroundtab';
-import { registerErrorInstrumentation } from './errors';
 import { MetricsInstrumentation } from './metrics';
 import {
   defaultRequestInstrumentionOptions,
@@ -171,9 +170,6 @@ export class BrowserTracing implements Integration {
       startTransactionOnLocationChange,
     );
 
-    // TODO: Should this be default behaviour?
-    registerErrorInstrumentation();
-
     if (markBackgroundTransactions) {
       registerBackgroundTabDetection();
     }

packages/tracing/src/browser/errors.ts renamed to packages/tracing/src/errors.ts

@@ -1,7 +1,7 @@
 import { addInstrumentationHandler, logger } from '@sentry/utils';
 
-import { SpanStatus } from '../spanstatus';
-import { getActiveTransaction } from './utils';
+import { getActiveTransaction } from './browser/utils';
+import { SpanStatus } from './spanstatus';
 
 /**
  * Configures global error listeners

packages/tracing/src/hubextensions.ts

@@ -1,6 +1,7 @@
 import { getMainCarrier, Hub } from '@sentry/hub';
 import { TransactionContext } from '@sentry/types';
 
+import { registerErrorInstrumentation } from './errors';
 import { IdleTransaction } from './idletransaction';
 import { Transaction } from './transaction';
 
@@ -63,9 +64,9 @@ export function startIdleTransaction(
 }
 
 /**
- * This patches the global object and injects the Tracing extensions methods
+ * @private
  */
-export function addExtensionMethods(): void {
+export function _addTracingExtensions(): void {
   const carrier = getMainCarrier();
   if (carrier.__SENTRY__) {
     carrier.__SENTRY__.extensions = carrier.__SENTRY__.extensions || {};
@@ -77,3 +78,13 @@ export function addExtensionMethods(): void {
     }
   }
 }
+
+/**
+ * This patches the global object and injects the Tracing extensions methods
+ */
+export function addExtensionMethods(): void {
+  _addTracingExtensions();
+
+  // If an error happens globally, we should make sure transaction status is set to error.
+  registerErrorInstrumentation();
+}

packages/tracing/test/browser/request.test.ts

@@ -35,7 +35,9 @@ jest.mock('@sentry/utils', () => {
       if (type === 'xhr') {
         mockXHRCallback = jest.fn(callback);
       }
-      return mockAddInstrumentationHandler({ callback, type });
+      if (typeof mockAddInstrumentationHandler === 'function') {
+        return mockAddInstrumentationHandler({ callback, type });
+      }
     },
   };
 });

packages/tracing/test/browser/errors.test.ts renamed to packages/tracing/test/errors.test.ts

@@ -1,9 +1,9 @@
 import { BrowserClient } from '@sentry/browser';
 import { Hub, makeMain } from '@sentry/hub';
 
-import { SpanStatus } from '../../src';
-import { registerErrorInstrumentation } from '../../src/browser/errors';
-import { addExtensionMethods } from '../../src/hubextensions';
+import { SpanStatus } from '../src';
+import { registerErrorInstrumentation } from '../src/errors';
+import { _addTracingExtensions } from '../src/hubextensions';
 
 const mockAddInstrumentationHandler = jest.fn();
 let mockErrorCallback: () => void = () => undefined;
@@ -19,13 +19,15 @@ jest.mock('@sentry/utils', () => {
       if (type === 'unhandledrejection') {
         mockUnhandledRejectionCallback = callback;
       }
-      return mockAddInstrumentationHandler({ callback, type });
+      if (typeof mockAddInstrumentationHandler === 'function') {
+        return mockAddInstrumentationHandler({ callback, type });
+      }
     },
   };
 });
 
 beforeAll(() => {
-  addExtensionMethods();
+  _addTracingExtensions();
 });
 
 describe('registerErrorHandlers()', () => {