invertase / react-native-firebase

πŸ”₯ A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.66k stars 2.21k forks source link

[πŸ›] πŸ”₯ Wrong crash attributes on iOS #7835

Closed luqas11 closed 2 months ago

luqas11 commented 3 months ago

Issue

When sending two non fatal reports with recordError() and some attributes, the first report has the attributes of the second in the Crashlytics dashboard. On Android the behavior is correct (i.e. the first and second report have their corresponding attributes), this is an iOS only issue.

To test it, I set up an empty app with two buttons to send two error reports:

/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 *
 * @format
 */

import React, {useEffect} from 'react';
import {Button, SafeAreaView} from 'react-native';

import crashlytics from '@react-native-firebase/crashlytics';

function App(): React.JSX.Element {
  useEffect(() => {
    crashlytics().log('Example log');
  }, []);

  return (
    <SafeAreaView>
      <Button
        title="Send Error 1"
        onPress={() => {
          crashlytics().setAttribute('some_attribute', 'value1');
          crashlytics().setAttribute('another_attribute', 'value');
          crashlytics().recordError(new Error('Error 1'));
        }}
      />
      <Button
        title="Send Error 2"
        onPress={() => {
          crashlytics().setAttribute('some_attribute', 'value2');
          crashlytics().recordError(new Error('Error 2'));
        }}
      />
    </SafeAreaView>
  );
}

export default App;

On iOS, if I press the "Send Error 1" button, then press the "Send Error 2" button, and restart the app, I get the following records on the dashboard:

For Error 1:

image image

For Error 2:

image image

As can be seen in the screenshots, the first report has some_attribute set to value2, despite its being set to value1 right before calling recordError(). Doing exactly the same on Android, I get the right behavior:

For Error 1:

image image

For Error 2:

image image

The "Error 1" report has some_attribute set to value1 as expected.

Is this difference between platforms a bug, or an intended behavior? Thanks in advance.


Project Files

Javascript

Click To Expand

#### `package.json`: ```json { "name": "ExampleProjectCL", "version": "0.0.1", "private": true, "scripts": { "android": "react-native run-android", "ios": "react-native run-ios", "lint": "eslint .", "start": "react-native start", "test": "jest" }, "dependencies": { "@react-native-firebase/app": "^20.1.0", "@react-native-firebase/crashlytics": "^20.1.0", "react": "18.2.0", "react-native": "0.74.2" }, "devDependencies": { "@babel/core": "^7.20.0", "@babel/preset-env": "^7.20.0", "@babel/runtime": "^7.20.0", "@react-native/babel-preset": "0.74.84", "@react-native/eslint-config": "0.74.84", "@react-native/metro-config": "0.74.84", "@react-native/typescript-config": "0.74.84", "@types/react": "^18.2.6", "@types/react-test-renderer": "^18.0.0", "babel-jest": "^29.6.3", "eslint": "^8.19.0", "jest": "^29.6.3", "prettier": "2.8.8", "react-test-renderer": "18.2.0", "typescript": "5.0.4" }, "engines": { "node": ">=18" } } ``` #### `firebase.json` for react-native-firebase v6: ```json { "react-native": { "crashlytics_debug_enabled": true, "crashlytics_disable_auto_disabler": true, "crashlytics_auto_collection_enabled": true, "crashlytics_is_error_generation_on_js_crash_enabled": true, "crashlytics_javascript_exception_handler_chaining_enabled": true } } ```

iOS

Click To Expand

#### `ios/Podfile`: - [ ] I'm not using Pods - [x] I'm using Pods and my Podfile looks like: ```ruby # Resolve react_native_pods.rb with node to allow for hoisting require Pod::Executable.execute_command('node', ['-p', 'require.resolve( "react-native/scripts/react_native_pods.rb", {paths: [process.argv[1]]}, )', __dir__]).strip platform :ios, min_ios_version_supported prepare_react_native_project! linkage = ENV['USE_FRAMEWORKS'] if linkage != nil Pod::UI.puts "Configuring Pod with #{linkage}ally linked Frameworks".green use_frameworks! :linkage => linkage.to_sym end target 'ExampleProjectCL' do config = use_native_modules! use_frameworks! :linkage => :static $RNFirebaseAsStaticFramework = true use_react_native!( :path => config[:reactNativePath], # An absolute path to your application root. :app_path => "#{Pod::Config.instance.installation_root}/.." ) target 'ExampleProjectCLTests' do inherit! :complete # Pods for testing end post_install do |installer| # https://github.com/facebook/react-native/blob/main/packages/react-native/scripts/react_native_pods.rb#L197-L202 react_native_post_install( installer, config[:reactNativePath], :mac_catalyst_enabled => false, # :ccache_enabled => true ) end end ``` #### `AppDelegate.m`: ```objc #import "AppDelegate.h" #import #import @implementation AppDelegate - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions { [FIRApp configure]; self.moduleName = @"ExampleProjectCL"; // You can add your custom initial props in the dictionary below. // They will be passed down to the ViewController used by React Native. self.initialProps = @{}; return [super application:application didFinishLaunchingWithOptions:launchOptions]; } - (NSURL *)sourceURLForBridge:(RCTBridge *)bridge { return [self bundleURL]; } - (NSURL *)bundleURL { #if DEBUG return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index"]; #else return [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"]; #endif } @end ```


Android

Click To Expand

#### Have you converted to AndroidX? - [x] my application is an AndroidX application? - [x] I am using `android/gradle.settings` `jetifier=true` for Android compatibility? - [ ] I am using the NPM package `jetifier` for react-native compatibility? #### `android/build.gradle`: ```groovy buildscript { ext { buildToolsVersion = "34.0.0" minSdkVersion = 23 compileSdkVersion = 34 targetSdkVersion = 34 ndkVersion = "26.1.10909125" kotlinVersion = "1.9.22" } repositories { google() mavenCentral() } dependencies { classpath("com.android.tools.build:gradle") classpath("com.facebook.react:react-native-gradle-plugin") classpath("org.jetbrains.kotlin:kotlin-gradle-plugin") classpath 'com.google.gms:google-services:4.4.2' classpath 'com.google.firebase:firebase-crashlytics-gradle:3.0.0' } } apply plugin: "com.facebook.react.rootproject" ``` #### `android/app/build.gradle`: ```groovy apply plugin: "com.android.application" apply plugin: "org.jetbrains.kotlin.android" apply plugin: "com.facebook.react" apply plugin: 'com.android.application' apply plugin: 'com.google.gms.google-services' apply plugin: 'com.google.firebase.crashlytics' /** * This is the configuration block to customize your React Native Android app. * By default you don't need to apply any configuration, just uncomment the lines you need. */ react { /* Folders */ // The root of your project, i.e. where "package.json" lives. Default is '..' // root = file("../") // The folder where the react-native NPM package is. Default is ../node_modules/react-native // reactNativeDir = file("../node_modules/react-native") // The folder where the react-native Codegen package is. Default is ../node_modules/@react-native/codegen // codegenDir = file("../node_modules/@react-native/codegen") // The cli.js file which is the React Native CLI entrypoint. Default is ../node_modules/react-native/cli.js // cliFile = file("../node_modules/react-native/cli.js") /* Variants */ // The list of variants to that are debuggable. For those we're going to // skip the bundling of the JS bundle and the assets. By default is just 'debug'. // If you add flavors like lite, prod, etc. you'll have to list your debuggableVariants. // debuggableVariants = ["liteDebug", "prodDebug"] /* Bundling */ // A list containing the node command and its flags. Default is just 'node'. // nodeExecutableAndArgs = ["node"] // // The command to run when bundling. By default is 'bundle' // bundleCommand = "ram-bundle" // // The path to the CLI configuration file. Default is empty. // bundleConfig = file(../rn-cli.config.js) // // The name of the generated asset file containing your JS bundle // bundleAssetName = "MyApplication.android.bundle" // // The entry file for bundle generation. Default is 'index.android.js' or 'index.js' // entryFile = file("../js/MyApplication.android.js") // // A list of extra flags to pass to the 'bundle' commands. // See https://github.com/react-native-community/cli/blob/main/docs/commands.md#bundle // extraPackagerArgs = [] /* Hermes Commands */ // The hermes compiler command to run. By default it is 'hermesc' // hermesCommand = "$rootDir/my-custom-hermesc/bin/hermesc" // // The list of flags to pass to the Hermes compiler. By default is "-O", "-output-source-map" // hermesFlags = ["-O", "-output-source-map"] } /** * Set this to true to Run Proguard on Release builds to minify the Java bytecode. */ def enableProguardInReleaseBuilds = false /** * The preferred build flavor of JavaScriptCore (JSC) * * For example, to use the international variant, you can use: * `def jscFlavor = 'org.webkit:android-jsc-intl:+'` * * The international variant includes ICU i18n library and necessary data * allowing to use e.g. `Date.toLocaleString` and `String.localeCompare` that * give correct results when using with locales other than en-US. Note that * this variant is about 6MiB larger per architecture than default. */ def jscFlavor = 'org.webkit:android-jsc:+' android { ndkVersion rootProject.ext.ndkVersion buildToolsVersion rootProject.ext.buildToolsVersion compileSdk rootProject.ext.compileSdkVersion namespace "com.exampleprojectcl" defaultConfig { applicationId "com.exampleprojectcl" minSdkVersion rootProject.ext.minSdkVersion targetSdkVersion rootProject.ext.targetSdkVersion versionCode 1 versionName "1.0" } signingConfigs { debug { storeFile file('debug.keystore') storePassword 'android' keyAlias 'androiddebugkey' keyPassword 'android' } } buildTypes { debug { signingConfig signingConfigs.debug } release { // Caution! In production, you need to generate your own keystore file. // see https://reactnative.dev/docs/signed-apk-android. signingConfig signingConfigs.debug minifyEnabled enableProguardInReleaseBuilds proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro" } } } dependencies { // The version of react-native is set by the React Native Gradle Plugin implementation("com.facebook.react:react-android") if (hermesEnabled.toBoolean()) { implementation("com.facebook.react:hermes-android") } else { implementation jscFlavor } } apply from: file("../../node_modules/@react-native-community/cli-platform-android/native_modules.gradle"); applyNativeModulesAppBuildGradle(project) ``` #### `android/settings.gradle`: ```groovy rootProject.name = 'ExampleProjectCL' apply from: file("../node_modules/@react-native-community/cli-platform-android/native_modules.gradle"); applyNativeModulesSettingsGradle(settings) include ':app' includeBuild('../node_modules/@react-native/gradle-plugin') ``` #### `MainApplication.kt`: ```kotlin package com.exampleprojectcl import com.facebook.react.ReactActivity import com.facebook.react.ReactActivityDelegate import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint.fabricEnabled import com.facebook.react.defaults.DefaultReactActivityDelegate class MainActivity : ReactActivity() { /** * Returns the name of the main component registered from JavaScript. This is used to schedule * rendering of the component. */ override fun getMainComponentName(): String = "ExampleProjectCL" /** * Returns the instance of the [ReactActivityDelegate]. We use [DefaultReactActivityDelegate] * which allows you to enable New Architecture with a single boolean flags [fabricEnabled] */ override fun createReactActivityDelegate(): ReactActivityDelegate = DefaultReactActivityDelegate(this, mainComponentName, fabricEnabled) } ``` #### `AndroidManifest.xml`: ```xml ```


Environment

Click To Expand

**`react-native info` output:** ``` System: OS: macOS 13.5.2 CPU: (8) arm64 Apple M1 Memory: 377.42 MB / 16.00 GB Shell: version: "5.9" path: /bin/zsh Binaries: Node: version: 20.9.0 path: ~/.nvm/versions/node/v20.9.0/bin/node Yarn: version: 3.6.4 path: /opt/homebrew/bin/yarn npm: version: 10.1.0 path: ~/.nvm/versions/node/v20.9.0/bin/npm Watchman: version: 2023.10.09.00 path: /opt/homebrew/bin/watchman Managers: CocoaPods: version: 1.13.0 path: /Users/username/.rbenv/shims/pod SDKs: iOS SDK: Platforms: - DriverKit 23.0 - iOS 17.0 - macOS 14.0 - tvOS 17.0 - watchOS 10.0 Android SDK: Not Found IDEs: Android Studio: 2022.3 AI-223.8836.35.2231.10811636 Xcode: version: 15.0/15A240d path: /usr/bin/xcodebuild Languages: Java: version: 17.0.11 path: /usr/bin/javac Ruby: version: 3.0.6 path: /Users/username/.rbenv/shims/ruby npmPackages: "@react-native-community/cli": Not Found react: installed: 18.2.0 wanted: 18.2.0 react-native: installed: 0.74.2 wanted: 0.74.2 react-native-macos: Not Found npmGlobalPackages: "*react-native*": Not Found Android: hermesEnabled: true newArchEnabled: false iOS: hermesEnabled: true newArchEnabled: false ``` - **Platform that you're experiencing the issue on**: - [x] iOS - [ ] Android - [ ] **iOS** but have not tested behavior on Android - [ ] **Android** but have not tested behavior on iOS - [ ] Both - **`react-native-firebase` version you're using that has this issue:** - `20.1.0` - **`Firebase` module(s) you're using that has the issue:** - `Crashlytics` - **Are you using `TypeScript`?** - `Y` & `5.0.4`


mikehardy commented 3 months ago

Interesting -

1- javascript implementation is here, does some basic argument validation then defers immediately to native:

https://github.com/invertase/react-native-firebase/blob/d849667a1b3614c4a938c1f2bea892758831b368/packages/crashlytics/lib/index.js#L76-L90

2- java implementation similarly does a basic validation that crashlytics is enabled at all, then delegates immediately to firebase-android-sdk:

https://github.com/invertase/react-native-firebase/blob/d849667a1b3614c4a938c1f2bea892758831b368/packages/crashlytics/android/src/main/java/io/invertase/firebase/crashlytics/ReactNativeFirebaseCrashlyticsModule.java#L118-L124

3- objective-c also just verifies that crashlytics is enabled then sets the attribute via immediate delegate call to firebase-ios-sdk method:

https://github.com/invertase/react-native-firebase/blob/d849667a1b3614c4a938c1f2bea892758831b368/packages/crashlytics/ios/RNFBCrashlytics/RNFBCrashlyticsModule.m#L128-L137

By deduction: one of the attributes from report 1 or report 2 would be missing if the argument validation logic or crashlytics enabled verification logic were false, but the attributes are both set (and incorrectly mingled on iOS) so we may infer that we are definitely calling into firebase-ios-sdk with the setAttribute calls

Given there is nothing else happening in this layer of the stack, I think this points to an issue in firebase-ios-sdk

I believe the best next step is to make a similar reproduction using the Crashlytics quickstart https://github.com/firebase/quickstart-ios/tree/main/crashlytics and logging an issue in firebase-ios-sdk repo pointing at the issue there as I think that's where the fault is located

Note especially that if you run the quickstart app under Xcode there is a debugger attached which interferes with Crashlytics: https://github.com/firebase/quickstart-ios/tree/main/crashlytics#trigger-a-crash-in-crashlytics-quickstart-app - this causes a lot of confusion sometimes. You have to launch it manually, not under Xcode

Adding the setAttributes calls (and another crash button) is hopefully a trivial extension to the quickstart so the repro is fast - there is example code included in my code permalinks above

luqas11 commented 3 months ago

Hi @mikehardy! Thanks for the quick response.

It appears that your deduction was right. I took the Crashlytics quickstart project, duplicated the crash button, and added the error attributes:

struct CrashButtonView: View {
  var body: some View {
    NavigationView {
        VStack {
            Button(action: {
                let crashlyticsReference = Crashlytics.crashlytics()
                crashlyticsReference.setCustomValue("value1", forKey: "some_attribute")
                crashlyticsReference.setCustomValue("value", forKey: "another_attribute")
                let error = NSError(domain: NSURLErrorDomain, code: -1001, userInfo: nil)
                Crashlytics.crashlytics().record(error: error)
            }) {
                Text("Send Error 1")
            }
            Button(action: {
                let crashlyticsReference = Crashlytics.crashlytics()
                crashlyticsReference.setCustomValue("value2", forKey: "some_attribute")
                let error = NSError(domain: NSURLErrorDomain, code: -1002, userInfo: nil)
                Crashlytics.crashlytics().record(error: error)
            }) {
                Text("Send Error 2")
            }
        }
      .navigationTitle("Crashlytics Example")
    }
  }
}

And, after repeating the experiment, I can see that the bug persists:

For Error 1:

image

For Error 2:

image

Both reports have some_attribute set to value2, despite its being set to value1 for the first one. I will open an issue at firebase-ios-sdk and wait for their response.

Thanks again for the analysis!

luqas11 commented 3 months ago

Here is the firebase-sdk-ios issue that I created, for future reference: https://github.com/firebase/firebase-ios-sdk/issues/13124

mikehardy commented 3 months ago

Reproductions like you've done are worth more than all the words I can put in a comment box, so your effort is the real motive force here, thanks for pursuing it upstream and hopefully there's a quick resolution - cheers

github-actions[bot] commented 2 months ago

Hello πŸ‘‹, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.