fix(rum-angular): capture angular navigation errors (#1662)

Author: david-luna

docs/reference/agent-api.md

@@ -130,7 +130,7 @@ apm.addFilter(function (payload) {
 ```
 
 ::::{note}
-The payload will be dropped if one of the filters return a falsy value.
+The payload will be dropped if one of the filters return a falsy value or if after applying all filters the transactions and errors arrays are empty.
 ::::
 
 

package.json

@@ -195,7 +195,6 @@
     "@elastic/apm-rum-vue": "file:packages/rum-vue"
   },
   "engines": {
-    "node": ">=12 <=16",
-    "npm": "6"
+    "node": ">=20.0.0"
   }
 }

packages/rum-angular/src/apm.service.ts

@@ -59,6 +59,7 @@ export class ApmService {
   }
 
   observe() {
+    const apm = this.apm
     let transaction
     this.router.events.subscribe(event => {
       const eventName = event.toString()
@@ -69,6 +70,11 @@ export class ApmService {
           canReuse: true
         })
       } else if (eventName.indexOf('NavigationError') >= 0) {
+        // @ts-expect-error - error prop not defined in types for this angular version
+        const err = event.error
+        if (err) {
+          apm.captureError(err)
+        }
         transaction && transaction.detectFinish()
       } else if (eventName.indexOf('NavigationEnd') >= 0) {
         if (!transaction) {

packages/rum-angular/test/specs/apm.service.spec.ts

@@ -24,8 +24,8 @@
  */
 
 import { TestBed, ComponentFixture } from '@angular/core/testing'
-import { NgModule, Component } from '@angular/core'
-import { Routes, Router } from '@angular/router'
+import { NgModule, Component, Injectable } from '@angular/core'
+import { Routes, Router, Resolve } from '@angular/router'
 import { RouterTestingModule } from '@angular/router/testing'
 import { Location } from '@angular/common'
 import { ApmBase } from '@elastic/apm-rum'
@@ -77,11 +77,25 @@ class SlugComponent {}
 })
 class AppComponent {}
 
+@Injectable()
+class ResolveBogus implements Resolve<any> {
+  resolve() {
+    throw new Error('Method not implemented.')
+  }
+}
+
 const routes: Routes = [
   { path: '', redirectTo: 'home', pathMatch: 'full' },
   { path: 'home', component: HomeComponent },
   { path: 'lazy', loadChildren: () => LazyModule },
-  { path: 'slug/:id', component: SlugComponent }
+  { path: 'slug/:id', component: SlugComponent },
+  {
+    path: 'error-route',
+    component: SlugComponent,
+    resolve: {
+      data: ResolveBogus
+    }
+  }
 ]
 
 describe('ApmService', () => {
@@ -95,7 +109,7 @@ describe('ApmService', () => {
     TestBed.configureTestingModule({
       imports: [ApmModule, RouterTestingModule.withRoutes(routes)],
       declarations: [HomeComponent, AppComponent, SlugComponent],
-      providers: [ApmService]
+      providers: [ApmService, ResolveBogus]
     }).compileComponents()
 
     TestBed.overrideProvider(APM, {
@@ -248,5 +262,34 @@ describe('ApmService', () => {
         })
       })
     })
+
+    it('should capture navigation errors', done => {
+      spyOn(service.apm, 'startTransaction').and.callThrough()
+      spyOn(service.apm, 'captureError')
+      const tr = service.apm.getCurrentTransaction()
+
+      fixture.ngZone.run(() => {
+        router.navigate(['/error-route']).then(
+          () => {
+            throw new Error('navigation should fail')
+          },
+          navError => {
+            expect(location.path()).toBe('/')
+            expect(service.apm.startTransaction).toHaveBeenCalledWith(
+              '/error-route',
+              'route-change',
+              {
+                managed: true,
+                canReuse: true
+              }
+            )
+            expect(service.apm.captureError).toHaveBeenCalledWith(navError)
+            expect(tr.name).toEqual('/error-route')
+
+            done()
+          }
+        )
+      })
+    })
   })
 })

packages/rum-core/src/common/config-service.js

@@ -26,6 +26,7 @@
 import { getCurrentScript, setLabel, merge, extend, isUndefined } from './utils'
 import EventHandler from './event-handler'
 import { CONFIG_CHANGE, LOCAL_CONFIG_KEY } from './constants'
+import { __DEV__ } from '../state'
 
 function getConfigFromScript() {
   var script = getCurrentScript()
@@ -124,11 +125,23 @@ class Config {
   }
 
   applyFilters(data) {
-    for (var i = 0; i < this.filters.length; i++) {
-      data = this.filters[i](data)
-      if (!data) {
+    try {
+      for (var i = 0; i < this.filters.length; i++) {
+        data = this.filters[i](data)
+        if (!data) {
+          return
+        }
+      }
+      data.transactions = data.transactions.filter(t => t)
+      data.errors = data.errors.filter(e => e)
+      if (data.transactions.length === 0 && data.errors.length === 0) {
         return
       }
+    } catch (e) {
+      if (__DEV__) {
+        console.log('[Elastic APM] Error applying filters for RUM payload', e)
+      }
+      return
     }
     return data
   }

packages/rum-core/test/common/config-service.spec.js

@@ -68,29 +68,54 @@ describe('ConfigService', function () {
   })
 
   it('should addFilter correctly', function () {
+    var rawPayload = { transactions: [{}], errors: [{}] }
+    var filterPayload1 = { transactions: [{}, {}], errors: [{}, {}] }
+    var filterPayload2 = { transactions: [{}, {}, {}], errors: [{}, {}, {}] }
     expect(function () {
       configService.addFilter('test')
     }).toThrow()
 
     configService.addFilter(function (testArg) {
-      expect(testArg).toBe('hamid-test')
-      return 'hamid-test-1'
+      expect(testArg).toBe(rawPayload)
+      return filterPayload1
     })
 
     configService.addFilter(function (testArg) {
-      expect(testArg).toBe('hamid-test-1')
-      return 'hamid-test-2'
+      expect(testArg).toBe(filterPayload1)
+      return filterPayload2
     })
 
-    var result = configService.applyFilters('hamid-test')
-    expect(result).toBe('hamid-test-2')
+    var result = configService.applyFilters(rawPayload)
+    expect(result).toBe(filterPayload2)
 
     configService.addFilter(function () {})
     configService.addFilter(function () {
       throw new Error('Out of reach!')
     })
 
-    result = configService.applyFilters('hamid-test')
+    result = configService.applyFilters(rawPayload)
+    expect(result).toBeUndefined()
+  })
+
+  it('should applyFilters without throwing', function () {
+    var rawPayload = { transactions: [{}], errors: [{}] }
+
+    configService.addFilter(function () {
+      throw new Error('Should now throw!')
+    })
+
+    var result = configService.applyFilters(rawPayload)
+    expect(result).toBeUndefined()
+  })
+
+  it('should applyFilters and filter empty payloads', function () {
+    var rawPayload = { transactions: [{}], errors: [{}] }
+
+    configService.addFilter(function () {
+      return { transactions: [], errors: [] }
+    })
+
+    var result = configService.applyFilters(rawPayload)
     expect(result).toBeUndefined()
   })
 

packages/rum/src/index.d.ts

@@ -187,7 +187,29 @@ interface ServiceFactory {
 }
 
 type FilterFn = (payload: Payload) => Payload | boolean | void
-type Payload = { [key: string]: any }
+type SpanPayload = {
+  id: string
+  type: string
+  name: string
+}
+type TransactionPayload = {
+  id: string
+  type: string
+  name: string
+  spans: Partial<SpanPayload>[]
+}
+type ErrorPayload = {
+  id: string
+  exception: {
+    type: string
+    message: string
+  }
+}
+type Payload = {
+  transactions: Partial<TransactionPayload>[]
+  errors: Partial<ErrorPayload>[]
+  [key: string]: any
+}
 
 type TaskId = string | number
 type LabelValue = string | number | boolean