Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐛] Bug Report Title - App crashed on first time launch with latest version [v10.8.1] #4979

Closed
5 of 10 tasks
JiboStore opened this issue Mar 3, 2021 · 10 comments · Fixed by #4986 or #4990
Closed
5 of 10 tasks
Labels
help: needs-triage Issue needs additional investigation/triaging. impact: crash Behaviour causing app to crash. type: bug New bug report Workflow: Waiting for User Response Blocked waiting for user response.

Comments

@JiboStore
Copy link
Contributor

JiboStore commented Mar 3, 2021

Issue

Upon upgrading to latest version (v10.8.1), first install and run of the app causes crash / hang (if debug version)

import firebase from '@react-native-firebase/app';
firebase.initializeApp({appId: 'com.testrefresh'}); // this line causes the problem

This issue only happens on Android 11 devices (API Level 30)
-. start a new emulator with API Level 30 and run it there to see the first time launch crash:
emulator @Nexus_6_API_30 -wipe-data

-. It does not happen on lower Android versions
-. It does not happen on second and subsequent launches
-. there's no log at all other than WIN DEATH and process killed
-. the difference between working and non-working is just the react-native-firebase package version

The same code was fine before upgrading (prior version was v8.4.0)

I have created 2 branches (1 good case, and 1 bad case) based on the initial react-native init blank project setup, with just react-native-firebase dependencies being added together with the initializeApp code

Prior to upgrade (working code):
https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/tree/rnfirebase/repro-good

After upgrade (crashing)
https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/tree/rnfirebase/repro-bad

The difference between ok and crashing code:
https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/compare/rnfirebase/repro-good?expand=1

Update:

Re-run again and get this suspicious callstack in logcat.
Could this be the cause?
https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/blob/rnfirebase/repro-bad-logcat/logcat.txt#L311

There are no changes at all, just the react-native-firebase versions


Project Files

Javascript

Click To Expand

package.json:

# N/A

firebase.json for react-native-firebase v6:

# N/A

iOS

Click To Expand

ios/Podfile:

  • I'm not using Pods
  • I'm using Pods and my Podfile looks like:
# N/A

AppDelegate.m:

// N/A


Android

Click To Expand

Have you converted to AndroidX?

  • my application is an AndroidX application?
  • 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:

// N/A

android/app/build.gradle:

// N/A

android/settings.gradle:

// N/A

MainApplication.java:

// N/A

AndroidManifest.xml:

<!-- N/A -->


Environment

Click To Expand

react-native info output:

 OUTPUT GOES HERE
  • Platform that you're experiencing the issue on:
    • 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:
    • e.g. 5.4.3
  • Firebase module(s) you're using that has the issue:
    • e.g. Instance ID
  • Are you using TypeScript?
    • Y/N & VERSION


@JiboStore JiboStore added help: needs-triage Issue needs additional investigation/triaging. type: bug New bug report labels Mar 3, 2021
@mikehardy
Copy link
Collaborator

Could you please provide a crash stack trace?

@andersonaddo andersonaddo added impact: crash Behaviour causing app to crash. Workflow: Waiting for User Response Blocked waiting for user response. labels Mar 3, 2021
@JiboStore
Copy link
Contributor Author

Sorry, previous run do not have any suspicious crash stack except win death
But I tried re-run again and get something suspicious

https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/blob/rnfirebase/repro-bad-logcat/logcat.txt#L311

@mikehardy
Copy link
Collaborator

Very strange! Is it possible to reproduce this startup crash initializing a second app with a clean known-good project from https://github.com/mikehardy/rnfbdemo/blob/master/make-demo.sh ?

JiboStore added a commit to JiboStore/rnfirebase-firstcrash-repro-210303 that referenced this issue Mar 4, 2021
@JiboStore
Copy link
Contributor Author

@mikehardy thanks for the help~

I run your make-demo.sh and the generated project does not have the issue, however I noticed that there is no firebase.initializeApp() function call anywhere, so I added it in App.js like this:

JiboStore/rnfirebase-firstcrash-repro-210303@4edb2aa

then the issue happens

The same suspected trace is logged in 2 trial runs (across 2 separate installs, since issue only happens on first run of new installation):

https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/blob/rnfbdemo/master/logcat.txt#L4604
https://github.com/JiboStore/rnfirebase-firstcrash-repro-210303/blob/rnfbdemo/master/logcat-2.txt#L314

@mikehardy
Copy link
Collaborator

Thanks - I really appreciate the logcats, and knowing you can reproduce it with the make-demo script is great, I've always got that script set up on my machine for troubleshooting problems in this repo. I can check it out this morning

@mikehardy
Copy link
Collaborator

Okay, I reproduce the crash (on iOS as well!) when I use initializeApp that way.

My question is, why would you attempt to use the API that way? It does not seem to conform to the method signature at all?

https://rnfirebase.io/reference/app#initializeApp

You want to send in an argument with a different shape, specifically I think you want { name: 'your name here' } ?

I have successful tests when I use it like that, e.g., these work for me:

xit('it should initialize dynamic apps', function () {
const name = `testscoreapp${FirebaseHelpers.id}`;
const platformAppConfig = FirebaseHelpers.app.config();
return firebase.initializeApp(platformAppConfig, name).then(newApp => {
newApp.name.should.equal(name);
newApp.toString().should.equal(name);
newApp.options.apiKey.should.equal(platformAppConfig.apiKey);
return newApp.delete();
});
});

(some side notes as a maintainer, I will fix all of these: 1) the type docs indicate return value of FirebaseApp for the initializeApp API call, but it should be Promise<FirebaseApp>, 2) the e2e tests here were disabled and not structured well, I have them patched up and working locally 3) the module of course should not crash with unexpected input - it works with correct input! - but I will patch up error handling)

@mikehardy
Copy link
Collaborator

Okay

  • I was not able to reliably reproduce the crash so I can't make any changes to the native modules
  • The javascript lib does an incredible amount of validation but the argument order is very important, you were attempting to provide a config value (which should be the second argument) as the first argument (which should be an options value, followed by your config). If the arguments are not in the order or shape expected, the javascript initializeApp code makes some assumptions and tries to initialize the default app

My proposed fix for you was to use name object property vs appId, but what you really want if you want to initialize appId is to pass options as well as a name as the second argument:

https://github.com/invertase/react-native-firebase/pull/4986/files#diff-ba4ac6d78adbedb6c794417f7f99411bf18deab1aa254662d8e778eacb49e55aR67

At which point you will receive an error because you need to send in the rest of the properties as well (apiKey etc etc): https://github.com/invertase/react-native-firebase/pull/4986/files#diff-ba4ac6d78adbedb6c794417f7f99411bf18deab1aa254662d8e778eacb49e55aR70

Also as mentioned you will want to await the call since initializeApp should return a Promise (which I have fixed in the typings)

Since this was an API usage issue, not really a bug, and I can't make it crash in testing when used correctly or incorrectly, I don't think there is anything else actionable here.

@JiboStore
Copy link
Contributor Author

Hi, sorry to bring this up again,

From your comments, here are further things I realised and some clarifications I'd like to make:
-. From the documentation, both Signature 1 and Signature 2 of initializeApp indicates that the 2nd param are optional, initially I was skipping them and only provide the FirebaseAppOptions as the first param:
initializeApp(options: FirebaseAppOptions, config?: FirebaseAppConfig): FirebaseApp;
initializeApp(options: FirebaseAppOptions, name?: undefined | string): FirebaseApp;
However, I tried filling in 2nd params accordingly (tried with Signature 1 and Signature 2) but the problem persist

-. I realised that I was missing another required properties of FirebaseAppOptions, that is projectId, so I modified the initializeApp calls to this:
firebase.initializeApp({appId: 'com.testrefresh', projectId: 'project-id-from-firebase-console'});
This also doesn't solve the problem

-. I only have the 1 app and just realised that I can forego calling that method entirely, accessing firebase.firestore() still let me access firestore services in old version (v8.4.1) after omitting the initializeApp calls and app name is initialized as '[DEFAULT]'

-. In v10.8.1, omitting the initializeApp call also result in the issue, running the second time, I am able to access firestore services without crash

Digging into the source-code and diff-ing v.8.4.1 and v10.8.1, I found this change:
-. https://github.com/invertase/react-native-firebase/blob/master/packages/app/android/src/reactnative/java/io/invertase/firebase/utils/ReactNativeFirebaseUtilsModule.java#L164
-. https://github.com/invertase/react-native-firebase/blob/%40react-native-firebase/app%408.4.1/packages/app/android/src/reactnative/java/io/invertase/firebase/utils/ReactNativeFirebaseUtilsModule.java#L164

Further digging into context.getExternalFilesDir(null) I found that it may indeed return null, and when it does so, the app will crash with the stated logcat
-. https://developer.android.com/reference/android/content/Context.html#getExternalFilesDir(java.lang.String)

After I added the null check, it works fine!

@mikehardy mikehardy reopened this Mar 5, 2021
@mikehardy
Copy link
Collaborator

Ah ha! You appear to have found the native crash I couldn't trigger - since you already have the change, would you mind posting a PR for it? I could get it merged in and released quickly

And no - if you just have one app, there is no need to formally initialize it - the "default" app will always be initialized for you with no effort on your part, from the google-services.json (or plist, for iOS) files

@JiboStore
Copy link
Contributor Author

Thanks! I've cleaned up my code and created a PR for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help: needs-triage Issue needs additional investigation/triaging. impact: crash Behaviour causing app to crash. type: bug New bug report Workflow: Waiting for User Response Blocked waiting for user response.
Projects
None yet
3 participants