fix(angular): Stop routing spans on navigation cancel and error events (#8369)

Previously, in our Angular routing instrumentation, we only stopped
routing spans when a navigation ended successfully. This was because we
only listened to
[`NavigationEnd`](https://angular.io/api/router/NavigationEnd) router
events. However, the Angular router emits other events in unsuccessful
routing attempts:
* a routing error (e.g. unknown route) emits
[`NavigationError`](https://angular.io/api/router/NavigationCancel)
* a cancelled navigation (e.g. due to a route guard that
rejected/returned false) emits
[`NavigationCancel`](https://angular.io/api/router/NavigationCancel)

This fix adjusts our instrumentation to also listen to these events and
to stop the span accordingly.

Author: Lms24

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/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();
+          });
       });
     });
   }