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

docs(firebase_messaging): Update example app & documentation for initializing Firebase app from top-level. #8061

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

russellwheatley
Copy link
Member

Description

Replace this paragraph with a description of what this PR is doing. If you're modifying existing behavior, describe the existing behavior, how this PR is changing it, and what motivated the change.

Related Issues

fixes #6087

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@github-actions
Copy link

github-actions bot commented Feb 10, 2022

Visit the preview URL for this PR (updated for commit f7a5e9d):

https://flutter-firebase-docs--pr8061-russell-messaging-f-v7uc3sjd.web.app

(expires Thu, 17 Feb 2022 14:35:13 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@russellwheatley russellwheatley changed the title docs(firebase_messaging): Update example app & documentation for initializing Firebase app from global. docs(firebase_messaging): Update example app & documentation for initializing Firebase app from top-level. Feb 10, 2022
@russellwheatley russellwheatley added type: documentation Improvements or additions to documentation plugin: messaging blocked: do-not-merge PR blocked externally labels Jul 11, 2022
@russellwheatley
Copy link
Member Author

russellwheatley commented Jul 11, 2022

Decided to solve this at the root. I believe there are too many connections/writes at the same time to the SQL instance hence the error. Possibly need to cache Firebase or Firestore instance if initializing app from separate isolate.

@russellwheatley
Copy link
Member Author

Closing as the issue appears to be resolved.

@deepak786
Copy link
Contributor

deepak786 commented Jan 17, 2023

@russellwheatley Is it resolved from the flutter firebase package itself?

@russellwheatley
Copy link
Member Author

@deepak786 The issue is closed so I presumed it was resolved. Are you still experiencing the issue?

@deepak786
Copy link
Contributor

@russellwheatley we are using the approach defined in this PR. So I want to know if we should continue with that or not.

@russellwheatley
Copy link
Member Author

@deepak786 I'm not sure, I presumed the lack of responses to the issue, and that it was closed meant it had been resolved at the level of the SDK. I think I'd prefer to reopen the issue if it is still a problem.

@Mayb3Nots
Copy link

Mayb3Nots commented Feb 7, 2023

@deepak786 I'm not sure, I presumed the lack of responses to the issue, and that it was closed meant it had been resolved at the level of the SDK. I think I'd prefer to reopen the issue if it is still a problem.

Hi @russellwheatley, Im getting same error message in Crashlytics today. I saw that you mentioned fixing it at the root where the SQL instance seemed to be overwhelm, is that still planned or do I have to implement the fix suggested by this PR?

@Lyokone since you closed the issue could you give some information on the reason?

With further investigation I found another crash log in Crashlytics.

java.lang.RuntimeException: Failed to gain exclusive lock to the Cloud Firestore client's offline persistence. This generally means you are using Cloud Firestore from multiple processes in your app. Keep in mind that multi-process Android apps execute the code in your Application class in all processes, so you may need to avoid initializing Cloud Firestore in your Application class. If you are intentionally using Cloud Firestore from multiple processes, you can only enable offline persistence (that is, call setPersistenceEnabled(true)) in one of them.

From this error message it seems like we shouldn't be initializing two instances of Firebase (Assuming its creating a firestore instance under the hood). If we do so I think we need to set FirebaseFirestore.instance.enablePersistence(PersistenceSettings(synchronizeTabs: false)) after initialising in our isolate (Havent test it so).

@giorgio79
Copy link

Hope making initapp a global will solve the originating issue:
#6087

I reworked my app based on the Firebase Messaging Flutter example app, and with two initilizeApp calls I started seeing the referenced github issue...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked: do-not-merge PR blocked externally plugin: messaging type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [cloud_firestore] database is locked (Sqlite code 5 SQLITE_BUSY)
5 participants