Functions custom HttpsError regression from v8 to v9

[mikehardy]

Step 0: Are you in the right place?

• For issues or feature requests related to the code in this repository
  file a GitHub issue.
  • If this is a feature request please use the Feature Request template.
• For general technical questions, post a question on StackOverflow
  with the firebase tag.
• For general (non-iOS) Firebase discussion, use the firebase-talk
  google group.
• For backend issues, console issues, and other non-SDK help that does not fall under one
  of the above categories, reach out to
  Firebase Support.
• Once you've read this section and determined that your issue is appropriate for
  this repository, please delete this section.

[REQUIRED] Step 1: Describe your environment

• Xcode version: 13.4
• Firebase SDK version: 9.1.0
• Installation method: CocoaPods (select one)
• Firebase Component: Functions
• Target platform(s): iOS

[REQUIRED] Step 2: Describe the problem

I'm forward-porting react-native-firebase to firebase-ios-sdk v9+ and I noticed a regression in custom error message behavior.

Prior to v9 here, if a function threw a custom HttpsError, the custom error details would be reflected in the error returned from the SDK, but with v9 all custom errors appear to be mapped to 'INTERNAL', which is the generic HTTP 500 response code mapping

Steps to reproduce:

Code up a simple test cloud function that will return an error if you supply it with nothing, or use ours:

https://github.com/invertase/react-native-firebase/blob/main/.github/workflows/scripts/functions/src/testFunctionDefaultRegion.ts

https://github.com/invertase/react-native-firebase/blob/52715959a375e8babd129baec38356b78ce80ab6/.github/workflows/scripts/functions/src/testFunctionDefaultRegion.ts#L42-L45

  const { type, asError, inputData } = data;
  if (!Object.hasOwnProperty.call(SAMPLE_DATA, type)) {
    throw new functions.https.HttpsError('invalid-argument', 'Invalid test requested.');
  }

Call that cloud function, with no arguments so it will return the custom error:

https://github.com/invertase/react-native-firebase/blob/52715959a375e8babd129baec38356b78ce80ab6/packages/functions/e2e/functions.e2e.js#L175-L185

    it('errors return instance of HttpsError', async function () {
      const functionRunner = firebase.functions().httpsCallable('testFunctionDefaultRegion');


      try {
        await functionRunner({});
        return Promise.reject(new Error('Function did not reject with error.'));
      } catch (e) {
        should.equal(e.details, null);
        e.code.should.equal('invalid-argument');
        e.message.should.equal('Invalid test requested.');
      }

Relevant Code:

...which goes to this Objective-C code:

https://github.com/invertase/react-native-firebase/blob/52715959a375e8babd129baec38356b78ce80ab6/packages/functions/ios/RNFBFunctions/RNFBFunctionsModule.m#L43-L81

  NSURL *url = [NSURL URLWithString:customUrlOrRegion];
  FIRFunctions *functions =
      (url && url.scheme && url.host)
          ? [FIRFunctions functionsForApp:firebaseApp customDomain:customUrlOrRegion]
          : [FIRFunctions functionsForApp:firebaseApp region:customUrlOrRegion];


  if (host != nil) {
    [functions useEmulatorWithHost:host port:[port intValue]];
  }


  FIRHTTPSCallable *callable = [functions HTTPSCallableWithName:name];


  if (options[@"timeout"]) {
    callable.timeoutInterval = [options[@"timeout"] doubleValue];
  }


  [callable callWithObject:[wrapper valueForKey:@"data"]
                completion:^(FIRHTTPSCallableResult *_Nullable result, NSError *_Nullable error) {
                  if (error) {
                    NSObject *details = [NSNull null];
                    NSString *message = error.localizedDescription;
                    NSMutableDictionary *userInfo = [NSMutableDictionary dictionary];
                    if ([error.domain isEqual:@"com.firebase.functions"]) {
                      details = error.userInfo[@"details"];
                      if (details == nil) {
                        details = [NSNull null];
                      }
                    }


                    userInfo[@"code"] = [self getErrorCodeName:error];
                    userInfo[@"message"] = message;
                    userInfo[@"details"] = details;


                    [RNFBSharedUtils rejectPromiseWithUserInfo:reject userInfo:userInfo];
                  } else {
                    resolve(@{@"data" : [result data]});
                  }
                }];
}

and just returns internal instead of the expected invalid-argument code with description from the function. firebase-android-sdk still returns the custom code/message

I threw a log message in to get the raw error from the SDK immediately after it falls into the if(error) case:

NSLog(@"FIREBASE FUNCTIONS ERROR: %@", error);

which yields

2022-05-27 01:03:03.602 Df testing[54318:76b6d] FIREBASE FUNCTIONS ERROR: Error Domain=com.firebase.functions Code=13 "INTERNAL" UserInfo={NSLocalizedDescription=INTERNAL}

Looks like something about the Obj-C --> Swift port missed custom error handling somehow?

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[mikehardy]

@paulb777 good news bad news: bad news, yes another v9 issue, but good news: this and #9849 are the only two things I see in our nearly 100% coverage e2e test harness, going from v8 to v9. Every other test is passing, I take that as a good sign

[paulb777]

@mikehardy Thanks again for the detailed report. I'll take a look.

For the next Swift API migration, let's try to coordinate running your tests before we release. 😃

[mikehardy]

You can always do it :-)
https://github.com/invertase/react-native-firebase/blob/main/tests/README.md
https://rnfirebase.io/#ios (to force a new cocoapods version)

Or just ask of course

[paulb777]

I'm not able to reproduce the issue in our JavaScript-based test harness in #9857. We're still generating FIRFunctionsErrorCodeInvalidArgument the same way we did in 8.x.

It's not immediately clear how to run the TypeScript test. Our integration test infrastructure seems to rely on having an index.js file and I couldn't get https://github.com/invertase/react-native-firebase/blob/main/.github/workflows/scripts/start-firebase-emulator.sh to start. After installing a few things, I still see authentication errors.

Any suggestions about the best way to get to an Objective-C reproducible example? Whether it's to adapt our integration tests to handle TypeScript, change something about the TypeScript failure case and add it to index.js, or something else.

cc: @inlined

[mikehardy]

I couldn't get https://github.com/invertase/react-native-firebase/blob/main/.github/workflows/scripts/start-firebase-emulator.sh to start

What's the error there? It attempts to compile the functions first, then start the emulator, but we run that in CI via github actions all the time, it must just need a small amount of percussive maintenance somewhere to get it moving...

[mikehardy]

maybe change this

firebase-ios-sdk/FirebaseFunctions/Backend/index.js

Line 108 in c2bc549

response.status(400).send();

to this as in our harness functions? did you already try that?

    throw new functions.https.HttpsError('invalid-argument', 'Invalid test requested.');

[paulb777]

Thanks! I can now reproduce. I also had to change from onRequest to onCall:

This function in index.js generates a different error on Firebase 8 versus 9, just as described in the OP.

exports.httpErrorTest = functions.https.onCall((data) => {
  throw new functions.https.HttpsError('invalid-argument', 'Invalid test requested.');
});

My JavaScript is weak and TypeScript non-existent, so I appreciate the pointer. 😄

On the other path, I'm still running into issues after installing yarn, starting the Firebase CLI, repeatedly finding and killing hung processes on ports, and running npm run build in the functions directory - https://gist.github.com/paulb777/a31e45bbf3237154488a4213584045dd. But will drop this, now that I can repro in index.js.

[paulb777]

This one is a definite breakage in the Obj-C to Swift port. I'm most of the way to a fix and should have a PR up by early next week.