-
Notifications
You must be signed in to change notification settings - Fork 927
Analytics: problem with "prevent initialization in unsupported environments" handling #3573
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
Comments
I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight. |
To answer your questions:
Unfortunately this makes the developer have to handle asynchronous initialization if they want to use We can look into a better way to address this. We can switch to using warnings for the sync checks, which already block initialization and won't lead to any further errors. Unfortunately it will be a little trickier to block the code that leads to the native browser error (which I'm sure you also don't want to see in your logs) after the async IndexedDB.open() check fails but there might be a way around it. Will try to look into these. |
I see, thanks for all the details. I believe we should definitely do something at firebase level. I am not sure how everyone codes their calls to analytics but usually the calls to log events will be "everywhere" in controllers and methods associated with user actions.
As an example of what it looks like on production, you can visit our app at https://battletraders.io with firefox in incognito. You will have an error report right away. |
I will definitely look into doing a warning instead of an error when we can prevent initialization but just have one question: were you not getting errors from these same browsers before? This same error should have happened in all previous versions of analytics, except without being wrapped by Analytics text, so it would just say: "A mutation operation was attempted on a database that did now allow mutations." This operation has always been called when analytics initializes. The only change should have been that we now try this same operation before initialization and throw a more detailed error if it doesn't work. |
@hsubox76 Sorry for the late reply. I was out for a week. I found an older error that confirms what you are saying.
I also found this other error, cookie related this time, that is also throwing an error and rather be a warning.
Suggestion:
The concept would be something like: const {isSupported, reason} = await analytics.checkEnvSupport()
if (!isSupported) {
if (reason === 'analytics/cookies-not-enabled') {
alertUser('Please enable cookies')
}
// other reasons...
} |
I have a similar error. I am integrating an app of react with firebase (notifications and analytics) but analytics dont work. The browser is Chrome and the cookis are enabled provider.ts:108 Uncaught FirebaseError: Analytics: Cookies are not enabled in this browser environment. Analytics requires cookies to be enabled. (analytics/cookies-not-enabled). |
Addressing the cookie issue in the new issue you opened here: #3749 |
Describe your environment
Describe the problem
Errors are thrown when the environment is unsupported
Steps to reproduce:
firebase.analytics()
)Will throw:
Questions
Why does it need to throw an error in the first place? Can't this silently fail with a warning in the console.
We get hundreds of thousands of error reports because of this.
Why do we have to run
isSupported
ourselves? can't the firebase lib check and initialize if it can? We could configure it to throw an error or just log a warning if some people do want to catch it but I think it would be better if it didn't break by default.isSupported()
is async, why? This makes it difficult to properly setup everything without delaying or queuing any call to analytics methods.For example if it were not async we could just do something like
The text was updated successfully, but these errors were encountered: