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.

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