diff --git a/packages/react-native/android/src/main/java/backtrace/library/BacktraceReactNative.java b/packages/react-native/android/src/main/java/backtrace/library/BacktraceReactNative.java index 2229f014..e1da9701 100644 --- a/packages/react-native/android/src/main/java/backtrace/library/BacktraceReactNative.java +++ b/packages/react-native/android/src/main/java/backtrace/library/BacktraceReactNative.java @@ -15,6 +15,7 @@ import android.content.Context; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -32,6 +33,9 @@ public class BacktraceReactNative extends ReactContextBaseJavaModule { private static final boolean nativeLibraryLoaded = loadNativeLibrary(); + // The crash handler's start-up attributes outrank any later update of the same key in the uploaded report. + private static final Set HANDLER_ATTRIBUTE_NAMES = new HashSet<>(Arrays.asList("guid", "error.type")); + public native void Crash(); private final Context context; @@ -77,13 +81,13 @@ public Boolean initialize(String minidumpSubmissionUrl, String databasePath, Rea } Map attributes = readableAttributes != null ? readableAttributes.toHashMap() : new HashMap(); - String[] keys = new String[attributes.size()]; - String[] values = new String[attributes.size()]; - int index = 0; + List keys = new ArrayList<>(); + List values = new ArrayList<>(); for (Map.Entry attribute : attributes.entrySet()) { - keys[index] = attribute.getKey(); - values[index] = stringValue(attribute.getValue()); - index++; + if (HANDLER_ATTRIBUTE_NAMES.contains(attribute.getKey())) { + keys.add(attribute.getKey()); + values.add(stringValue(attribute.getValue())); + } } List attachments = stringList(attachmentPaths); @@ -93,8 +97,8 @@ public Boolean initialize(String minidumpSubmissionUrl, String databasePath, Rea minidumpSubmissionUrl, databasePath, crashHandlerConfiguration.getClassPath(), - keys, - values, + keys.toArray(new String[0]), + values.toArray(new String[0]), attachments.toArray(new String[0]), crashHandlerConfiguration.getCrashHandlerEnvironmentVariables(this.context.getApplicationInfo()).toArray(new String[0]) ); @@ -104,6 +108,12 @@ public Boolean initialize(String minidumpSubmissionUrl, String databasePath, Rea return false; } + for (Map.Entry attribute : attributes.entrySet()) { + if (!HANDLER_ATTRIBUTE_NAMES.contains(attribute.getKey())) { + BacktraceDatabase.addAttribute(attribute.getKey(), stringValue(attribute.getValue())); + } + } + this.registeredAttachments.addAll(attachments); this.initialized = true; return true; diff --git a/packages/react-native/src/BacktraceClient.ts b/packages/react-native/src/BacktraceClient.ts index b21050ee..4166d502 100644 --- a/packages/react-native/src/BacktraceClient.ts +++ b/packages/react-native/src/BacktraceClient.ts @@ -307,6 +307,7 @@ export class BacktraceClient extends BacktraceCoreClient } private initializeNativeCrashReporter(): CrashReporter | undefined { + this._crashReporter?.dispose(); if (!this.options.database?.enable) { return; } diff --git a/packages/react-native/src/crashReporter/CrashReporter.ts b/packages/react-native/src/crashReporter/CrashReporter.ts index da4b8c5e..bee668a4 100644 --- a/packages/react-native/src/crashReporter/CrashReporter.ts +++ b/packages/react-native/src/crashReporter/CrashReporter.ts @@ -16,15 +16,20 @@ export class CrashReporter { */ private static initialized = false; + private static activeReporter?: CrashReporter; + private static nativeSubmissionUrl?: string; + private static nativeDatabasePath?: string; + private static nativeAttributeNames = new Set(); + public initialize( submissionUrl: string, databasePath: string, attributes: Record, attachments: readonly BacktraceAttachment[], ): boolean { + const nativeDatabasePath = `${databasePath}/native`; if (CrashReporter.initialized) { - warnFailure("native crash reporting keeps the first client's configuration"); - return false; + return this.takeOver(submissionUrl, nativeDatabasePath, attributes, attachments); } if (!CrashReporter.BacktraceReactNative) { warnFailure('native crash reporting is off, BacktraceReactNative is not linked'); @@ -36,7 +41,7 @@ export class CrashReporter { return false; } - const nativeDatabasePath = `${databasePath}/native`; + const nativeAttributes = this.convertAttributes(attributes); try { this._fileSystem.createDirSync(nativeDatabasePath); @@ -45,7 +50,7 @@ export class CrashReporter { submissionUrl, nativeDatabasePath, { - ...this.convertAttributes(attributes), + ...nativeAttributes, 'error.type': 'Crash', }, this.convertAttachments(attachments), @@ -59,8 +64,11 @@ export class CrashReporter { return false; } - this._enabled = true; CrashReporter.initialized = true; + CrashReporter.nativeSubmissionUrl = submissionUrl; + CrashReporter.nativeDatabasePath = nativeDatabasePath; + CrashReporter.nativeAttributeNames = new Set(Object.keys(nativeAttributes)); + this.enable(); return true; } @@ -68,7 +76,7 @@ export class CrashReporter { if (!this._enabled) { return; } - this.update(() => CrashReporter.BacktraceReactNative.useAttributes(this.convertAttributes(attributes))); + this.useAttributes(this.convertAttributes(attributes)); } public updateAttachments(attachments: readonly BacktraceAttachment[]) { @@ -96,6 +104,49 @@ export class CrashReporter { public dispose(): void { this._enabled = false; + if (CrashReporter.activeReporter === this) { + CrashReporter.activeReporter = undefined; + } + } + + private takeOver( + submissionUrl: string, + nativeDatabasePath: string, + attributes: Record, + attachments: readonly BacktraceAttachment[], + ): boolean { + if (CrashReporter.activeReporter) { + warnFailure('native crash reporting stays with the client that is still running, dispose it first'); + return false; + } + if ( + submissionUrl !== CrashReporter.nativeSubmissionUrl || + nativeDatabasePath !== CrashReporter.nativeDatabasePath + ) { + warnFailure( + "native crash reports keep the first client's submission url and database path until the app restarts", + ); + } + + const blankedPreviousAttributes = Object.fromEntries( + [...CrashReporter.nativeAttributeNames].map((name) => [name, '']), + ); + this.enable(); + this.useAttributes({ ...blankedPreviousAttributes, ...this.convertAttributes(attributes) }); + this.updateAttachments(attachments); + return true; + } + + private enable() { + this._enabled = true; + CrashReporter.activeReporter = this; + } + + private useAttributes(attributes: Record) { + for (const name of Object.keys(attributes)) { + CrashReporter.nativeAttributeNames.add(name); + } + this.update(() => CrashReporter.BacktraceReactNative.useAttributes(attributes)); } private update(fn: () => void) { diff --git a/packages/react-native/tests/nativeCrashReporterReinitializationTests.spec.ts b/packages/react-native/tests/nativeCrashReporterReinitializationTests.spec.ts new file mode 100644 index 00000000..fa796d59 --- /dev/null +++ b/packages/react-native/tests/nativeCrashReporterReinitializationTests.spec.ts @@ -0,0 +1,257 @@ +import { NativeModules } from 'react-native'; +import { promisify } from 'util'; +import { mockStreamFileSystem } from './_mocks/fileSystem'; + +// This package's jest config replaces the react-native preset's setupFiles. The real Platform throws. +jest.mock('react-native', () => ({ + NativeModules: {}, + Platform: { + OS: 'ios', + select: (options: Record) => (options.ios !== undefined ? options.ios : options.default), + }, +})); + +jest.mock('../src/common/platformHelper', () => ({ + version: () => '0.81.6', +})); + +const nativeMock = { + initialize: jest.fn(), + useAttributes: jest.fn(), + useAttachments: jest.fn(), + crash: jest.fn(), +}; + +// CrashReporter caches BacktraceReactNative in a static field. The mock has to land before the module loads. +NativeModules.BacktraceReactNative = nativeMock; +NativeModules.BacktraceDirectoryProvider = { applicationDirectory: () => '/' }; +(globalThis as unknown as { RN$Bridgeless: boolean }).RN$Bridgeless = true; + +/* eslint-disable @typescript-eslint/no-var-requires */ +const { BacktraceClient } = require('../src/BacktraceClient'); +const { BacktraceFileAttachment } = require('../src/attachment/BacktraceFileAttachment'); +const { CrashReporter } = require('../src/crashReporter/CrashReporter'); +/* eslint-enable @typescript-eslint/no-var-requires */ + +const nextTick = promisify(process.nextTick); + +const SUBMISSION_URL = 'https://submit.backtrace.io/universe/token/json'; + +interface ClientOptions { + userAttributes?: Record; + attachmentPaths?: string[]; + url?: string; + databasePath?: string; + breadcrumbs?: boolean; +} + +function createClient({ + userAttributes = {}, + attachmentPaths = [], + url = SUBMISSION_URL, + databasePath = '/backtrace', + breadcrumbs = false, +}: ClientOptions = {}) { + const fileSystem = mockStreamFileSystem(); + return new BacktraceClient({ + options: { + url, + database: { enable: true, captureNativeCrashes: true, path: databasePath }, + metrics: { enable: false }, + breadcrumbs: breadcrumbs ? { maximumBreadcrumbs: 4 } : { enable: false }, + userAttributes: { + application: 'nativeReinitialization', + 'application.version': '1.0.0', + ...userAttributes, + }, + attachments: attachmentPaths.map((path) => new BacktraceFileAttachment(fileSystem, path)), + }, + fileSystem, + }); +} + +function initializeClient(options: ClientOptions = {}) { + const client = createClient(options); + client.initialize(); + return client; +} + +// Every update carries the full scoped set rather than a delta. Merge the calls before asserting. +function attributesSentToNative(): Record { + return Object.assign({}, ...nativeMock.useAttributes.mock.calls.map((call) => call[0])); +} + +function attachmentPathsSentToNative(): string[] { + return nativeMock.useAttachments.mock.calls.flatMap((call) => call[0]); +} + +describe('BacktraceClient native crash reporter re-initialization', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(console, 'warn').mockImplementation(() => undefined); + // initialized is static, and would otherwise make initialize() a no-op for every test after the first. + (CrashReporter as unknown as { initialized: boolean }).initialized = false; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('Should pass the attributes of a client created after dispose to the native crash reporter', () => { + initializeClient({ userAttributes: { 'user.id': 'first-user' } }).dispose(); + nativeMock.useAttributes.mockClear(); + + initializeClient({ userAttributes: { 'user.id': 'second-user' } }); + + expect(nativeMock.initialize).toHaveBeenCalledTimes(1); + expect(attributesSentToNative()['user.id']).toBe('second-user'); + }); + + it('Should clear attributes only the disposed client had in the native crash reporter', () => { + const first = initializeClient({ + userAttributes: { 'user.id': 'first-user', 'user.email': 'first@example.com' }, + }); + first.addAttribute({ 'session.id': 'first-session' }); + first.dispose(); + nativeMock.useAttributes.mockClear(); + + initializeClient({ userAttributes: { 'user.id': 'second-user' } }); + + const forwarded = attributesSentToNative(); + expect(forwarded['user.email']).toBe(''); + expect(forwarded['session.id']).toBe(''); + expect(forwarded['application']).toBe('nativeReinitialization'); + }); + + it('Should pass the attachments of a client created after dispose to the native crash reporter', () => { + initializeClient({ attachmentPaths: ['/logs/first.log'] }).dispose(); + nativeMock.useAttachments.mockClear(); + + initializeClient({ attachmentPaths: ['/logs/second.log'] }); + + expect(attachmentPathsSentToNative()).toContain('/logs/second.log'); + expect(attachmentPathsSentToNative()).not.toContain('/logs/first.log'); + }); + + it('Should forward an attribute added to a client created after dispose to the native crash reporter', () => { + initializeClient({ userAttributes: { 'user.id': 'first-user' } }).dispose(); + const second = initializeClient({ userAttributes: { 'user.id': 'second-user' } }); + nativeMock.useAttributes.mockClear(); + + second.addAttribute({ 'session.id': 'second-session' }); + + expect(attributesSentToNative()['session.id']).toBe('second-session'); + }); + + it('Should forward breadcrumbs of a client created after dispose to the native crash reporter', async () => { + initializeClient({ breadcrumbs: true }).dispose(); + const second = initializeClient({ breadcrumbs: true }); + nativeMock.useAttributes.mockClear(); + nativeMock.useAttachments.mockClear(); + + for (let i = 0; i < 20; i++) { + second.breadcrumbs?.info(`breadcrumb-${i}`); + await nextTick(); + } + + expect(attachmentPathsSentToNative().some((path) => path.includes('breadcrumb'))).toBe(true); + expect(attributesSentToNative()['breadcrumbs.lastId']).toBeDefined(); + }); + + it('Should NOT forward attributes from the disposed client once a new client is initialized', () => { + const first = initializeClient({ userAttributes: { 'user.id': 'first-user' } }); + first.dispose(); + initializeClient({ userAttributes: { 'user.id': 'second-user' } }); + nativeMock.useAttributes.mockClear(); + + first.addAttribute({ 'user.id': 'first-user' }); + + expect(nativeMock.useAttributes).not.toHaveBeenCalled(); + }); + + it('Should forward attributes after the same client is disposed and initialized again', () => { + const client = initializeClient({ userAttributes: { 'user.id': 'first-user' } }); + client.dispose(); + client.initialize(); + nativeMock.useAttributes.mockClear(); + + client.addAttribute({ 'user.id': 'second-user' }); + + expect(attributesSentToNative()['user.id']).toBe('second-user'); + }); + + it('Should forward attributes after the same client is initialized twice without dispose', () => { + const client = initializeClient({ userAttributes: { 'user.id': 'first-user' } }); + client.initialize(); + nativeMock.useAttributes.mockClear(); + + client.addAttribute({ 'user.id': 'second-user' }); + + expect(attributesSentToNative()['user.id']).toBe('second-user'); + }); + + it('Should keep the native crash reporter on the first client while it is not disposed', () => { + const first = initializeClient({ userAttributes: { 'user.id': 'first-user' } }); + nativeMock.useAttributes.mockClear(); + nativeMock.useAttachments.mockClear(); + + const second = initializeClient({ + userAttributes: { 'user.id': 'second-user' }, + attachmentPaths: ['/logs/second.log'], + }); + expect(nativeMock.useAttributes).not.toHaveBeenCalled(); + expect(nativeMock.useAttachments).not.toHaveBeenCalled(); + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining('stays with the client that is still running'), + ); + + second.addAttribute({ 'session.id': 'second-session' }); + expect(nativeMock.useAttributes).not.toHaveBeenCalled(); + + first.addAttribute({ 'session.id': 'first-session' }); + expect(attributesSentToNative()['session.id']).toBe('first-session'); + }); + + it('Should keep the native crash reporter on the live client when a disposed client is initialized again', () => { + const first = initializeClient({ userAttributes: { 'user.id': 'first-user' } }); + first.dispose(); + const second = initializeClient({ userAttributes: { 'user.id': 'second-user' } }); + first.initialize(); + nativeMock.useAttributes.mockClear(); + + first.addAttribute({ 'session.id': 'first-session' }); + expect(nativeMock.useAttributes).not.toHaveBeenCalled(); + + second.addAttribute({ 'session.id': 'second-session' }); + expect(attributesSentToNative()['session.id']).toBe('second-session'); + }); + + it('Should warn when a client created after dispose uses a different submission url', () => { + initializeClient({ userAttributes: { 'user.id': 'first-user' } }).dispose(); + + initializeClient({ + userAttributes: { 'user.id': 'second-user' }, + url: 'https://submit.backtrace.io/other-universe/token/json', + }); + + expect(console.warn).toHaveBeenCalledWith(expect.stringContaining('submission url and database path')); + expect(attributesSentToNative()['user.id']).toBe('second-user'); + }); + + it('Should warn when a client created after dispose uses a different database path', () => { + initializeClient({ userAttributes: { 'user.id': 'first-user' } }).dispose(); + + initializeClient({ userAttributes: { 'user.id': 'second-user' }, databasePath: '/other-backtrace' }); + + expect(console.warn).toHaveBeenCalledWith(expect.stringContaining('submission url and database path')); + expect(attributesSentToNative()['user.id']).toBe('second-user'); + }); + + it('Should NOT warn when a client created after dispose uses the same configuration', () => { + initializeClient({ userAttributes: { 'user.id': 'first-user' } }).dispose(); + + initializeClient({ userAttributes: { 'user.id': 'second-user' } }); + + expect(console.warn).not.toHaveBeenCalled(); + }); +});