feat(crashlytics): collect Angular context when calling recordError directly (#9456)

* Collect Angular context when calling recordError directly

* Update attribute priority

---------

Co-authored-by: abrook <abrook@google.com>

Author: tonybaroneee

packages/crashlytics/src/angular/index.test.ts

@@ -37,6 +37,7 @@ import {
   BrowserTestingModule,
   platformBrowserTesting
 } from '@angular/platform-browser/testing';
+import { CrashlyticsService } from '../service';
 
 use(sinonChai);
 use(chaiAsPromised);
@@ -88,33 +89,31 @@ describe('FirebaseErrorHandler', () => {
   });
 
   it('should log the error to the console', async () => {
-    await router.navigate(['/static-route']);
-
     const testError = new Error('Test error message');
     errorHandler.handleError(testError);
     expect(getCrashlyticsStub).to.have.been.called;
-    expect(recordErrorStub).to.have.been.calledWith(
-      fakeCrashlytics,
-      testError,
-      {
-        'angular_route_path': '/static-route'
-      }
-    );
+    expect(recordErrorStub).to.have.been.calledWith(fakeCrashlytics, testError);
   });
 
-  it('should remove dynamic content from route', async () => {
-    await router.navigate(['/dynamic/my-name/route']);
+  describe('frameworkAttributesProvider', () => {
+    it('should report framework attributes', async () => {
+      await router.navigate(['/static-route']);
 
-    const testError = new Error('Test error message');
-    errorHandler.handleError(testError);
-    expect(recordErrorStub).to.have.been.called;
-    expect(recordErrorStub).to.have.been.calledWith(
-      fakeCrashlytics,
-      testError,
-      {
-        // eslint-disable-next-line camelcase
-        angular_route_path: '/dynamic/:id/route'
-      }
-    );
+      expect(
+        (fakeCrashlytics as CrashlyticsService).frameworkAttributesProvider!()
+      ).to.deep.equal({
+        'angular_route_path': '/static-route'
+      });
+    });
+
+    it('should remove dynamic content from route', async () => {
+      await router.navigate(['/dynamic/my-name/route']);
+
+      expect(
+        (fakeCrashlytics as CrashlyticsService).frameworkAttributesProvider!()
+      ).to.deep.equal({
+        'angular_route_path': '/dynamic/:id/route'
+      });
+    });
   });
 });

packages/crashlytics/src/angular/index.ts

@@ -23,6 +23,7 @@ import { registerCrashlytics } from '../register';
 import { recordError, getCrashlytics } from '../api';
 import { Crashlytics, CrashlyticsOptions } from '../public-types';
 import { FirebaseApp } from '@firebase/app';
+import { CrashlyticsService } from '../service';
 
 registerCrashlytics();
 
@@ -83,14 +84,22 @@ export class FirebaseErrorHandler implements ErrorHandler {
 
   constructor(app: FirebaseApp, crashlyticsOptions?: CrashlyticsOptions) {
     this.crashlytics = getCrashlytics(app, crashlyticsOptions);
+    (this.crashlytics as CrashlyticsService).frameworkAttributesProvider = () =>
+      this.getAttributes();
   }
 
   handleError(error: unknown): void {
-    const attributes = {
+    recordError(this.crashlytics, error);
+  }
+
+  /**
+   * Returns a record of framework-specific attributes based on the current application state to be
+   * attached to the error log.
+   */
+  private getAttributes(): Record<string, string> {
+    return {
       'angular_route_path': this.getSafeRoutePath(this.router)
     };
-
-    recordError(this.crashlytics, error, attributes);
   }
 
   /**

packages/crashlytics/src/api.test.ts

@@ -128,6 +128,7 @@ describe('Top level API', () => {
       value: originalCrypto,
       writable: true
     });
+    delete AUTO_CONSTANTS.appVersion;
   });
 
   describe('getCrashlytics()', () => {
@@ -338,6 +339,31 @@ describe('Top level API', () => {
       });
     });
 
+    it('should retrieve framework-specific attributes', () => {
+      const error = new Error('This is a test error');
+      error.stack = '...stack trace...';
+      error.name = 'TestError';
+
+      (fakeCrashlytics as CrashlyticsService).frameworkAttributesProvider =
+        () => ({
+          'framework_attr1': 'framework attribute #1',
+          'framework_attr2': 'framework attribute #2'
+        });
+
+      recordError(fakeCrashlytics, error);
+
+      expect(emittedLogs.length).to.equal(1);
+      const log = emittedLogs[0];
+      expect(log.attributes).to.deep.equal({
+        'error.type': 'TestError',
+        'error.stack': '...stack trace...',
+        [LOG_ENTRY_ATTRIBUTE_KEYS.APP_VERSION]: 'unset',
+        'framework_attr1': 'framework attribute #1',
+        'framework_attr2': 'framework attribute #2',
+        [LOG_ENTRY_ATTRIBUTE_KEYS.SESSION_ID]: MOCK_SESSION_ID
+      });
+    });
+
     describe('Session Metadata', () => {
       it('should retrieve existing session ID from sessionStorage', () => {
         storage[CRASHLYTICS_SESSION_ID_KEY] = 'existing-session-id';

packages/crashlytics/src/api.ts

@@ -83,7 +83,15 @@ export function recordError(
   const logger = (crashlytics as CrashlyticsInternal).loggerProvider.getLogger(
     'error-logger'
   );
-  const customAttributes = attributes || {};
+  const customAttributes: AnyValueMap = {};
+
+  // Add framework-specific metadata
+  const frameworkAttributesProvider = (crashlytics as CrashlyticsService)
+    .frameworkAttributesProvider;
+  if (frameworkAttributesProvider) {
+    const frameworkAttributes = frameworkAttributesProvider();
+    Object.assign(customAttributes, frameworkAttributes);
+  }
 
   // Add trace metadata
   const activeSpanContext = trace.getActiveSpan()?.spanContext();
@@ -107,6 +115,12 @@ export function recordError(
     customAttributes[LOG_ENTRY_ATTRIBUTE_KEYS.SESSION_ID] = sessionId;
   }
 
+  // Merge in any additional attributes. Explicitly provided attributes take precedence over
+  // automatically added attributes.
+  if (attributes) {
+    Object.assign(customAttributes, attributes);
+  }
+
   if (error instanceof Error) {
     logger.emit({
       severityNumber: SeverityNumber.ERROR,

packages/crashlytics/src/service.ts

@@ -21,6 +21,7 @@ import { LoggerProvider } from '@opentelemetry/sdk-logs';
 
 export class CrashlyticsService implements Crashlytics, _FirebaseService {
   private _options?: CrashlyticsOptions;
+  private _frameworkAttributesProvider?: () => Record<string, string>;
 
   constructor(public app: FirebaseApp, public loggerProvider: LoggerProvider) {}
 
@@ -35,4 +36,14 @@ export class CrashlyticsService implements Crashlytics, _FirebaseService {
   get options(): CrashlyticsOptions | undefined {
     return this._options;
   }
+
+  get frameworkAttributesProvider():
+    | (() => Record<string, string>)
+    | undefined {
+    return this._frameworkAttributesProvider;
+  }
+
+  set frameworkAttributesProvider(provider: () => Record<string, string>) {
+    this._frameworkAttributesProvider = provider;
+  }
 }