Importing core.ts can lead to unhandled rejections at global scope in restricted security contexts

[abierbaum]

Version info

Angular: 13.2.0

Firebase: 9.6.5-canary.0a04a1c06

AngularFire: 7.2.0

Other (e.g. Ionic/Cordova, Node, browser, operating system):

OS: Ubuntu 20.04.3 LTS
Node: 14.18.1

How to reproduce these conditions

Failing test unit, Stackblitz demonstrating the problem

N/A

Steps to set up and reproduce

The way to reproduce is to simply import angular/fire in such a way that angularfire/src/core.ts is evaluated.

If you look at these lines in core.ts: (https://github.com/angular/angularfire/blob/master/src/core.ts#L17)

globalThis[isAnalyticsSupportedPromiseSymbol] ||= isAnalyticsSupported().then(it =>
  globalThis[isAnalyticsSupportedValueSymbol] = it
);

globalThis[isMessagingSupportedPromiseSymbol] ||= isMessagingSupported().then(it =>
  globalThis[isMessagingSupportedValueSymbol] = it
);

globalThis[isRemoteConfigSupportedPromiseSymbol] ||= isRemoteConfigSupported().then(it =>
  globalThis[isRemoteConfigSupportedValueSymbol] = it
);

It looks like isAnalyticsSupported(), isMessagingSupported(), and isRemoteConfigSupported() will all three be called at import type with a Promise that gets evaluated but does not catch errors. This can cause problems when the code inside firebase that is doing the evaluation runs into a problem. You will end up with an unhandled rejection at the global application scope.

In my specific case, isRemoteConfigSupported is throwing an exception because the application is being run inside a restricted security context that can't access indexedDB, so this line: https://github.com/firebase/firebase-js-sdk/blob/b835b4cbabc4b7b180ae38b908c49205ce31a422/packages/util/src/environment.ts#L143 throws a browser SecurityError. The exception ends up propogating up to the promise evaluation above and is unhandled.

I may be misunderstanding how these methods are being used though so I am filing this bug to double check if this is expected behavior. If it isn't expected behavior then I suggest adding something that catches the exception and handles it. Maybe something like this:

globalThis[isRemoteConfigSupportedPromiseSymbol] ||= isRemoteConfigSupported().then(it =>
  globalThis[isRemoteConfigSupportedValueSymbol] = it;
).catch(() => {
  globalThis[isRemoteConfigSupportedValueSymbol] = false;
});

Sample data and security rules

N/A

Debug output

N/A

Expected behavior

I would expect that importing angular/fire will not run code that throws unhandled rejections.

Actual behavior

See above.

[jamesdaniels]

Thanks for reporting, IMO we should make sure that logic is guarded in the JS SDK. In the meantime I've created a PR to be a little bit more defensive in AngularFire.

[jamesdaniels]

Do you mind reporting over there?

[abierbaum]

@jamesdaniels Looks great to me. Thanks for the quick response.

[abierbaum]

Do you mind reporting over there?

I will check again, but IIRC I figured this out by finding a report over there of a case like this. It is just that for angular/fire it was being called globally so there was no great way to catch it.

[jamesdaniels]

Cutting 7.2.1 with the fix now