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

Remove error log from RNFirebaseRemoteConfig #836

Merged

Conversation

skovhus
Copy link
Contributor

@skovhus skovhus commented Feb 15, 2018

In case the app is offline and the consumer of this library is using Remote Config, it is perfectly fine to get a FIRRemoteConfigErrorInternalError error, but there is really no reason to show a red error while developing.

screenshot 2018-02-15 19 14 27

Instead, we should leave it up to the consumer of this library to handle any errors.

It seems that only that only RNFirebaseRemoteConfig.m should be fixed to not use RCTLogError where we also reject.

RNFirebaseMessaging.m is also using RCTLogError but actual errors where here is no completion handler with completionHandlerId.

Leave it up to the consumer of the library to handle any errors instead of forcing a red error screen. In case the app is offline, it is perfectly fine to get a FIRRemoteConfigErrorInternalError error.
@chrisbianca chrisbianca merged commit 25ad0f3 into invertase:master Feb 17, 2018
@chrisbianca
Copy link
Contributor

This seems reasonable, thanks for the PR

@skovhus skovhus deleted the remove-invasive-remote-config-log branch February 17, 2018 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants