From 4eb27089b7697b5c9f0a1c9df8d1e75c0cb972cf Mon Sep 17 00:00:00 2001 From: abrook Date: Thu, 18 Dec 2025 15:37:26 -0500 Subject: [PATCH 1/2] Collect Angular context when calling recordError directly --- .../crashlytics/src/angular/index.test.ts | 41 ++++++++++--------- packages/crashlytics/src/angular/index.ts | 15 +++++-- packages/crashlytics/src/api.test.ts | 26 ++++++++++++ packages/crashlytics/src/api.ts | 8 ++++ packages/crashlytics/src/service.ts | 11 +++++ 5 files changed, 78 insertions(+), 23 deletions(-) diff --git a/packages/crashlytics/src/angular/index.test.ts b/packages/crashlytics/src/angular/index.test.ts index 1a86b25f0a..1641c85a8b 100644 --- a/packages/crashlytics/src/angular/index.test.ts +++ b/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); @@ -93,28 +94,28 @@ describe('FirebaseErrorHandler', () => { 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' + }); + }); }); }); diff --git a/packages/crashlytics/src/angular/index.ts b/packages/crashlytics/src/angular/index.ts index aaae4b5931..35a110e697 100644 --- a/packages/crashlytics/src/angular/index.ts +++ b/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 { + return { 'angular_route_path': this.getSafeRoutePath(this.router) }; - - recordError(this.crashlytics, error, attributes); } /** diff --git a/packages/crashlytics/src/api.test.ts b/packages/crashlytics/src/api.test.ts index 3db8cee705..9c9f2d9337 100644 --- a/packages/crashlytics/src/api.test.ts +++ b/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'; diff --git a/packages/crashlytics/src/api.ts b/packages/crashlytics/src/api.ts index a55315a74f..3e039460e0 100644 --- a/packages/crashlytics/src/api.ts +++ b/packages/crashlytics/src/api.ts @@ -85,6 +85,14 @@ export function recordError( ); const customAttributes = attributes || {}; + // 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(); if (crashlytics.app.options.projectId && activeSpanContext?.traceId) { diff --git a/packages/crashlytics/src/service.ts b/packages/crashlytics/src/service.ts index b0712ba331..103a357711 100644 --- a/packages/crashlytics/src/service.ts +++ b/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; 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) + | undefined { + return this._frameworkAttributesProvider; + } + + set frameworkAttributesProvider(provider: () => Record) { + this._frameworkAttributesProvider = provider; + } } From e125f7a13b177513a996f62367864a6bd5f2e42f Mon Sep 17 00:00:00 2001 From: abrook Date: Thu, 18 Dec 2025 15:48:07 -0500 Subject: [PATCH 2/2] Update attribute priority --- packages/crashlytics/src/angular/index.test.ts | 2 -- packages/crashlytics/src/api.ts | 8 +++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/crashlytics/src/angular/index.test.ts b/packages/crashlytics/src/angular/index.test.ts index 1641c85a8b..2b0770d553 100644 --- a/packages/crashlytics/src/angular/index.test.ts +++ b/packages/crashlytics/src/angular/index.test.ts @@ -89,8 +89,6 @@ 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; diff --git a/packages/crashlytics/src/api.ts b/packages/crashlytics/src/api.ts index 3e039460e0..bc59f43f95 100644 --- a/packages/crashlytics/src/api.ts +++ b/packages/crashlytics/src/api.ts @@ -83,7 +83,7 @@ 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) @@ -115,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,