Merge branch 'develop' into kw-add-http-response-status-code

.github/workflows/build.yml

@@ -89,6 +89,7 @@ jobs:
               - 'scripts/**'
               - 'packages/core/**'
               - 'packages/tracing/**'
+              - 'packages/tracing-internal/**'
               - 'packages/utils/**'
               - 'packages/types/**'
               - 'packages/integrations/**'
@@ -424,7 +425,7 @@ jobs:
     name: Nextjs (Node ${{ matrix.node }}) Tests
     needs: [job_get_metadata, job_build]
     if: needs.job_get_metadata.outputs.changed_nextjs == 'true' || github.event_name != 'pull_request'
-    timeout-minutes: 15
+    timeout-minutes: 25
     runs-on: ubuntu-20.04
     strategy:
       fail-fast: false
@@ -684,7 +685,7 @@ jobs:
           yarn test
 
   job_remix_integration_tests:
-    name: Remix (Node ${{ matrix.node }}) Tests
+    name: Remix v${{ matrix.remix }} (Node ${{ matrix.node }}) Tests
     needs: [job_get_metadata, job_build]
     if: needs.job_get_metadata.outputs.changed_remix == 'true' || github.event_name != 'pull_request'
     runs-on: ubuntu-20.04
@@ -693,6 +694,7 @@ jobs:
       fail-fast: false
       matrix:
         node: [14, 16, 18]
+        remix: [1, 2]
     steps:
       - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
         uses: actions/checkout@v3
@@ -709,6 +711,7 @@ jobs:
       - name: Run integration tests
         env:
           NODE_VERSION: ${{ matrix.node }}
+          REMIX_VERSION: ${{ matrix.remix }}
         run: |
           cd packages/remix
           yarn test:integration:ci

packages/angular-ivy/src/sdk.ts

@@ -1,6 +1,6 @@
 import { VERSION } from '@angular/core';
 import type { BrowserOptions } from '@sentry/browser';
-import { init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
+import { defaultIntegrations, init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
 import { logger } from '@sentry/utils';
 
 import { IS_DEBUG_BUILD } from './flags';
@@ -21,6 +21,18 @@ export function init(options: BrowserOptions): void {
     version: SDK_VERSION,
   };
 
+  // Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`:
+  // TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a
+  // lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide.
+  // see:
+  //  - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097
+  //  - https://github.com/getsentry/sentry-javascript/issues/2744
+  if (options.defaultIntegrations === undefined) {
+    options.defaultIntegrations = defaultIntegrations.filter(integration => {
+      return integration.name !== 'TryCatch';
+    });
+  }
+
   checkAndSetAngularVersion();
   browserInit(options);
 }

packages/angular/src/sdk.ts

@@ -1,6 +1,6 @@
 import { VERSION } from '@angular/core';
 import type { BrowserOptions } from '@sentry/browser';
-import { init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
+import { defaultIntegrations, init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
 import { logger } from '@sentry/utils';
 
 import { IS_DEBUG_BUILD } from './flags';
@@ -21,6 +21,18 @@ export function init(options: BrowserOptions): void {
     version: SDK_VERSION,
   };
 
+  // Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`:
+  // TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a
+  // lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide.
+  // see:
+  //  - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097
+  //  - https://github.com/getsentry/sentry-javascript/issues/2744
+  if (options.defaultIntegrations === undefined) {
+    options.defaultIntegrations = defaultIntegrations.filter(integration => {
+      return integration.name !== 'TryCatch';
+    });
+  }
+
   checkAndSetAngularVersion();
   browserInit(options);
 }

packages/angular/src/tracing.ts

@@ -5,7 +5,7 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router
 // Duplicated import to work around a TypeScript bug where it'd complain that `Router` isn't imported as a type.
 // We need to import it as a value to satisfy Angular dependency injection. So:
 // eslint-disable-next-line @typescript-eslint/consistent-type-imports, import/no-duplicates
-import { Router } from '@angular/router';
+import { NavigationCancel, NavigationError, Router } from '@angular/router';
 // eslint-disable-next-line import/no-duplicates
 import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router';
 import { getCurrentHub, WINDOW } from '@sentry/browser';
@@ -131,7 +131,9 @@ export class TraceService implements OnDestroy {
   );
 
   public navEnd$: Observable<Event> = this._router.events.pipe(
-    filter(event => event instanceof NavigationEnd),
+    filter(
+      event => event instanceof NavigationEnd || event instanceof NavigationCancel || event instanceof NavigationError,
+    ),
     tap(() => {
       if (this._routingSpan) {
         runOutsideAngular(() => {

packages/angular/test/sdk.test.ts

@@ -1,6 +1,6 @@
 import * as SentryBrowser from '@sentry/browser';
 
-import { init } from '../src/sdk';
+import { defaultIntegrations, init } from '../src/index';
 
 describe('init', () => {
   it('sets the Angular version (if available) in the global scope', () => {
@@ -13,4 +13,33 @@ describe('init', () => {
     expect(setContextSpy).toHaveBeenCalledTimes(1);
     expect(setContextSpy).toHaveBeenCalledWith('angular', { version: 10 });
   });
+
+  describe('filtering out the `TryCatch` integration', () => {
+    const browserInitSpy = jest.spyOn(SentryBrowser, 'init');
+
+    beforeEach(() => {
+      browserInitSpy.mockClear();
+    });
+
+    it('filters if `defaultIntegrations` is not set', () => {
+      init({});
+
+      expect(browserInitSpy).toHaveBeenCalledTimes(1);
+
+      const options = browserInitSpy.mock.calls[0][0] || {};
+      expect(options.defaultIntegrations).not.toContainEqual(expect.objectContaining({ name: 'TryCatch' }));
+    });
+
+    it.each([false as const, defaultIntegrations])(
+      "doesn't filter if `defaultIntegrations` is set to %s",
+      defaultIntegrations => {
+        init({ defaultIntegrations });
+
+        expect(browserInitSpy).toHaveBeenCalledTimes(1);
+
+        const options = browserInitSpy.mock.calls[0][0] || {};
+        expect(options.defaultIntegrations).toEqual(defaultIntegrations);
+      },
+    );
+  });
 });

packages/angular/test/tracing.test.ts

@@ -1,5 +1,5 @@
 import { Component } from '@angular/core';
-import type { ActivatedRouteSnapshot } from '@angular/router';
+import type { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router';
 import type { Hub } from '@sentry/types';
 
 import { instrumentAngularRouting, TraceClassDecorator, TraceDirective, TraceMethodDecorator } from '../src';
@@ -185,6 +185,66 @@ describe('Angular Tracing', () => {
       env.destroy();
     });
 
+    it('finishes routing span on navigation error', async () => {
+      const customStartTransaction = jest.fn(defaultStartTransaction);
+
+      const env = await TestEnv.setup({
+        customStartTransaction,
+        routes: [
+          {
+            path: '',
+            component: AppComponent,
+          },
+        ],
+        useTraceService: true,
+      });
+
+      const finishMock = jest.fn();
+      transaction.startChild = jest.fn(() => ({
+        finish: finishMock,
+      }));
+
+      await env.navigateInAngular('/somewhere');
+
+      expect(finishMock).toHaveBeenCalledTimes(1);
+
+      env.destroy();
+    });
+
+    it('finishes routing span on navigation cancel', async () => {
+      const customStartTransaction = jest.fn(defaultStartTransaction);
+
+      class CanActivateGuard implements CanActivate {
+        canActivate(_route: ActivatedRouteSnapshot, _state: RouterStateSnapshot): boolean {
+          return false;
+        }
+      }
+
+      const env = await TestEnv.setup({
+        customStartTransaction,
+        routes: [
+          {
+            path: 'cancel',
+            component: AppComponent,
+            canActivate: [CanActivateGuard],
+          },
+        ],
+        useTraceService: true,
+        additionalProviders: [{ provide: CanActivateGuard, useClass: CanActivateGuard }],
+      });
+
+      const finishMock = jest.fn();
+      transaction.startChild = jest.fn(() => ({
+        finish: finishMock,
+      }));
+
+      await env.navigateInAngular('/cancel');
+
+      expect(finishMock).toHaveBeenCalledTimes(1);
+
+      env.destroy();
+    });
+
     describe('URL parameterization', () => {
       it.each([
         [

packages/angular/test/utils/index.ts

@@ -1,3 +1,4 @@
+import type { Provider } from '@angular/core';
 import { Component, NgModule } from '@angular/core';
 import type { ComponentFixture } from '@angular/core/testing';
 import { TestBed } from '@angular/core/testing';
@@ -47,6 +48,7 @@ export class TestEnv {
     startTransactionOnPageLoad?: boolean;
     startTransactionOnNavigation?: boolean;
     useTraceService?: boolean;
+    additionalProviders?: Provider[];
   }): Promise<TestEnv> {
     instrumentAngularRouting(
       conf.customStartTransaction || jest.fn(),
@@ -60,14 +62,16 @@ export class TestEnv {
     TestBed.configureTestingModule({
       imports: [AppModule, RouterTestingModule.withRoutes(routes)],
       declarations: [...(conf.components || []), AppComponent],
-      providers: useTraceService
+      providers: (useTraceService
         ? [
             {
               provide: TraceService,
               deps: [Router],
             },
+            ...(conf.additionalProviders || []),
           ]
-        : [],
+        : []
+      ).concat(...(conf.additionalProviders || [])),
     });
 
     const router: Router = TestBed.inject(Router);
@@ -80,10 +84,16 @@ export class TestEnv {
   public async navigateInAngular(url: string): Promise<void> {
     return new Promise(resolve => {
       return this.fixture.ngZone?.run(() => {
-        void this.router.navigateByUrl(url).then(() => {
-          this.fixture.detectChanges();
-          resolve();
-        });
+        void this.router
+          .navigateByUrl(url)
+          .then(() => {
+            this.fixture.detectChanges();
+            resolve();
+          })
+          .catch(() => {
+            this.fixture.detectChanges();
+            resolve();
+          });
       });
     });
   }

packages/browser-integration-tests/suites/public-api/captureException/classInstance/subject.js

@@ -0,0 +1,6 @@
+class MyTestClass {
+  prop1 = 'value1';
+  prop2 = 2;
+}
+
+Sentry.captureException(new MyTestClass());

packages/browser-integration-tests/suites/public-api/captureException/classInstance/test.ts

@@ -0,0 +1,21 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest('should capture an POJO', async ({ getLocalTestPath, page }) => {
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+  expect(eventData.exception?.values?.[0]).toMatchObject({
+    type: 'Error',
+    value: 'Object captured as exception with keys: prop1, prop2',
+    mechanism: {
+      type: 'generic',
+      handled: true,
+    },
+  });
+});

packages/browser-integration-tests/suites/public-api/captureException/empty_obj/test.ts → packages/browser-integration-tests/suites/public-api/captureException/emptyObj/test.ts

@@ -12,7 +12,7 @@ sentryTest('should capture an empty object', async ({ getLocalTestPath, page })
   expect(eventData.exception?.values).toHaveLength(1);
   expect(eventData.exception?.values?.[0]).toMatchObject({
     type: 'Error',
-    value: 'Non-Error exception captured with keys: [object has no keys]',
+    value: 'Object captured as exception with keys: [object has no keys]',
     mechanism: {
       type: 'generic',
       handled: true,

packages/browser-integration-tests/suites/public-api/captureException/errorEvent/init.js

@@ -0,0 +1,8 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  defaultIntegrations: false,
+});

packages/browser-integration-tests/suites/public-api/captureException/errorEvent/subject.js

@@ -0,0 +1,5 @@
+window.addEventListener('error', function (event) {
+  Sentry.captureException(event);
+});
+
+window.thisDoesNotExist();

packages/browser-integration-tests/suites/public-api/captureException/errorEvent/test.ts

@@ -0,0 +1,25 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest('should capture an ErrorEvent', async ({ getLocalTestPath, page, browserName }) => {
+  // On Firefox, the ErrorEvent has the `error` property and thus is handled separately
+  if (browserName === 'firefox') {
+    sentryTest.skip();
+  }
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+  expect(eventData.exception?.values?.[0]).toMatchObject({
+    type: 'ErrorEvent',
+    value: 'Event `ErrorEvent` captured as exception with message `Script error.`',
+    mechanism: {
+      type: 'generic',
+      handled: true,
+    },
+  });
+});

packages/browser-integration-tests/suites/public-api/captureException/event/subject.js

@@ -0,0 +1 @@
+Sentry.captureException(new Event('custom'));

packages/browser-integration-tests/suites/public-api/captureException/event/test.ts

@@ -0,0 +1,21 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest('should capture an Event', async ({ getLocalTestPath, page }) => {
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+  expect(eventData.exception?.values?.[0]).toMatchObject({
+    type: 'Event',
+    value: 'Event `Event` (type=custom) captured as exception',
+    mechanism: {
+      type: 'generic',
+      handled: true,
+    },
+  });
+});

packages/browser-integration-tests/suites/public-api/captureException/plainObject/subject.js

@@ -0,0 +1,4 @@
+Sentry.captureException({
+  prop1: 'value1',
+  prop2: 2,
+});