fix(app, secondary): reject if initializeApp fails on iOS

Previously the exceptions were swallowed, meaning that initialization failures were very difficult to diagnose, leading to developer confusion Fixes #5134
invertase · Apr 12, 2021 · d76eba3 · d76eba3
1 parent 245149c
commit d76eba3
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 2 deletions.
diff --git a/packages/app/e2e/app.e2e.js b/packages/app/e2e/app.e2e.js
@@ -53,21 +53,55 @@ describe('firebase', function () {
   });
 
   it('it should initialize dynamic apps', async function () {
+    const appCount = firebase.apps.length;
     const name = `testscoreapp${FirebaseHelpers.id}`;
     const platformAppConfig = FirebaseHelpers.app.config();
     const newApp = await firebase.initializeApp(platformAppConfig, name);
     newApp.name.should.equal(name);
     newApp.toString().should.equal(name);
     newApp.options.apiKey.should.equal(platformAppConfig.apiKey);
+    should.equal(firebase.apps.includes(firebase.app(name)), true);
+    should.equal(firebase.apps.length, appCount + 1);
     return newApp.delete();
   });
 
   it('should error if dynamic app initialization values are incorrect', async function () {
+    const appCount = firebase.apps.length;
     try {
       await firebase.initializeApp({ appId: 'myid' }, 'myname');
       throw new Error('Should have rejected incorrect initializeApp input');
     } catch (e) {
       e.message.should.equal("Missing or invalid FirebaseOptions property 'apiKey'.");
+      should.equal(firebase.apps.length, appCount);
+      should.equal(firebase.apps.includes('myname'), false);
+    }
+  });
+
+  it('should error if dynamic app initialization values are invalid', async function () {
+    // firebase-android-sdk will not complain on invalid initialization values, iOS throws
+    if (device.getPlatform() === 'android') {
+      return;
+    }
+
+    const appCount = firebase.apps.length;
+    try {
+      const firebaseConfig = {
+        apiKey: 'XXXXXXXXXXXXXXXXXXXXXXX',
+        authDomain: 'test-XXXXX.firebaseapp.com',
+        databaseURL: 'https://test-XXXXXX.firebaseio.com',
+        projectId: 'test-XXXXX',
+        storageBucket: 'tes-XXXXX.appspot.com',
+        messagingSenderId: 'XXXXXXXXXXXXX',
+        appId: '1:XXXXXXXXX',
+        app_name: 'TEST',
+      };
+      await firebase.initializeApp(firebaseConfig, 'myname');
+      throw new Error('Should have rejected incorrect initializeApp input');
+    } catch (e) {
+      e.code.should.containEql('app/unknown');
+      e.message.should.containEql('Configuration fails');
+      should.equal(firebase.apps.length, appCount);
+      should.equal(firebase.apps.includes('myname'), false);
     }
   });
 

diff --git a/packages/app/ios/RNFBApp/RNFBAppModule.m b/packages/app/ios/RNFBApp/RNFBAppModule.m
@@ -205,8 +205,7 @@ - (void)invalidate {
         firApp = [FIRApp appNamed:appName];
       }
     } @catch (NSException *exception) {
-      // TODO js error builder
-      // reject(@"code",@"message",[exception ]);
+      return [RNFBSharedUtils rejectPromiseWithExceptionDict:reject exception:exception];
     }
 
     firApp.dataCollectionDefaultEnabled = (BOOL) [appConfig valueForKey:@"automaticDataCollectionEnabled"];

diff --git a/packages/app/lib/internal/registry/app.js b/packages/app/lib/internal/registry/app.js
@@ -167,6 +167,8 @@ export function initializeApp(options = {}, configOrName) {
 
   const app = new FirebaseApp(options, { name }, false, deleteApp.bind(null, name, true));
 
+  // Note these initialization actions with side effects are performed prior to knowledge of
+  // successful initialization in the native code. Native code *may* throw an error.
   APP_REGISTRY[name] = app;
   onAppCreateFn(APP_REGISTRY[name]);
 
@@ -175,6 +177,16 @@ export function initializeApp(options = {}, configOrName) {
     .then(() => {
       app._initialized = true;
       return app;
+    })
+    .catch(e => {
+      // we need to clean the app entry from registry as the app does not actually exist
+      // There are still possible side effects from `onAppCreateFn` to consider but as existing
+      // code may rely on that function running prior to native create, re-ordering it is a semantic change
+      // and will be avoided
+      delete APP_REGISTRY[name];
+
+      // Now allow calling code to handle the initialization issue
+      throw e;
     });
 }