[cloud_firestore] Fixes crash on Android when run a transaction by hoc081098 · Pull Request #87 · firebase/flutterfire · GitHub

Description

Fixes crash on Android when run a transaction and without internet connection.
This occurred because:

Tasks.await(transactionTCSTask, timeout, TimeUnit.MILLISECONDS)
throws a Exception
→ result.error("Error performing transaction", e.getMessage(), null) will be called
→ transaction result is null
→ in addOnCompleteListener#onComplete:

    if (task.isSuccessful()) { // result is null
      result.success(task.getResult());
    }

will also be called
→ result#success has been called after result#error
→ This cause java.lang.IllegalStateException: Reply already submitted

Related Issues

fix #79

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.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
If the pull request affects only one plugin, the PR title starts with the name of the plugin in brackets (e.g. [cloud_firestore])
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 (flutter analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
I updated CHANGELOG.md to add a description of the change.
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 (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

@googlebot I signed it!

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Thanks for the PR! The cloud_firestore integration test crashed on this PR but fortunately the fix was easy. I've updated your PR with 928e052. (Yay tests!)

09-04 14:33:35.278  5207  5207 E AndroidRuntime: FATAL EXCEPTION: main
09-04 14:33:35.278  5207  5207 E AndroidRuntime: Process: io.flutter.plugins.firebase.firestoreexample, PID: 5207
09-04 14:33:35.278  5207  5207 E AndroidRuntime: java.lang.IllegalArgumentException: Unsupported value: io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$TransactionResult@d350ae3
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugin.common.StandardMessageCodec.writeValue(StandardMessageCodec.java:294)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugins.firebase.cloudfirestore.FirestoreMessageCodec.writeValue(CloudFirestorePlugin.java:886)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugin.common.StandardMethodCodec.encodeSuccessEnvelope(StandardMethodCodec.java:57)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler$1.success(MethodChannel.java:225)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$3.onComplete(CloudFirestorePlugin.java:446)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at com.google.android.gms.tasks.zzj.run(Unknown Source:4)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:873)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:99)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:193)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:6669)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
09-04 14:33:35.278  5207  5207 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

diff --git a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
index 71029b6a..eddabe3a 100644
--- a/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
+++ b/packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
@@ -443,7 +443,7 @@ public class CloudFirestorePlugin implements MethodCallHandler {
 
                       TransactionResult transactionResult = task.getResult();
                       if (transactionResult.exception == null) {
-                        result.success(transactionResult);
+                        result.success(transactionResult.result);
                       } else {
                         result.error(
                             "Error performing transaction",

I also made some minor fixes to the version number and reformatting.

Nice work :)

Wow. Thanks so much for this PR. I was seeing this crash for quite a while now. In Crashlytics it shows as the following:

DartMessenger.java line 35

io.flutter.embedding.engine.dart.DartMessenger$Reply.reply

This PR resolves #102.
It was just now that I figured out why this was happening as well. Lucky that I saw this PR before finishing my implementation - saving me some work. This shows that it really is benificial to create an issue with a PR or include a stack trace or similar in the PR (or issue) description so that it becomes searchable. I searched for the crash, but could not find anything.

…sh_android

@collinjackson Is there a reason this is not being merged?

Guys, do you have any progress?!
I have used cloud_firestore: 0.12.9+4 and I get crash of my app due 'offline' transaction every time!
When it can be retested again? thanks.

Just waiting 👍

Guys, do you have any progress?!
I have used cloud_firestore: 0.12.9+4 and I get crash of my app due 'offline' transaction every time!
When it can be retested again? thanks.

Just as a reminder, you can always use it like that:
cloud_firestore: git: url: https://github.com/FirebaseExtended/flutterfire.git path: packages/cloud_firestore ref: 2e7c183

Fixed, thanks guys!

…base#87) * Fix transaction crash on Android device * Update pub version * Bump for release * Reformat * Fix crash

I still get this crashing. Only on Android - LIVE devices and also simulators.

Basically occurs when I try and run a transaction when the device is offline

Crashalytics

CloudFirestorePlugin.java line 644

Caused by java.lang.AssertionError: INTERNAL ASSERTION FAILED: A transaction object cannot be used after its update callback has been invoked.

   at com.google.firebase.firestore.util.Assert.fail(com.google.firebase:firebase-firestore@@21.4.0:46)
   at com.google.firebase.firestore.util.Assert.hardAssert(com.google.firebase:firebase-firestore@@21.4.0:31)
   at com.google.firebase.firestore.core.Transaction.ensureCommitNotCalled(com.google.firebase:firebase-firestore@@21.4.0:246)
   at com.google.firebase.firestore.core.Transaction.write(com.google.firebase:firebase-firestore@@21.4.0:241)
   at com.google.firebase.firestore.core.Transaction.set(com.google.firebase:firebase-firestore@@21.4.0:106)
   at com.google.firebase.firestore.Transaction.set(com.google.firebase:firebase-firestore@@21.4.0:91)
   at com.google.firebase.firestore.Transaction.set(com.google.firebase:firebase-firestore@@21.4.0:67)
   at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$7.doInBackground(CloudFirestorePlugin.java:644)
   at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$7.doInBackground(CloudFirestorePlugin.java:638)
   at android.os.AsyncTask$2.call(AsyncTask.java:333)
   at java.util.concurrent.FutureTask.run(FutureTask.java:266)
   at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:245)
   at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
   at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
   at java.lang.Thread.run(Thread.java:764)

my android/app/build.gradle (Snapshot)

dependencies {
testImplementation 'junit:junit:4.12'
androidTestImplementation 'androidx.test:runner :1.1.1'
androidTestImplementation 'androidx.test.espresso:espresso-core: 3.1.1'
implementation 'com.google.firebase:firebase-analytics: 17.2.2'
implementation 'com.google.firebase:firebase-firestore: 21.4.0'
implementation 'com.google.firebase:firebase-crashlytics: 17.0.0-beta01'
}

Pubspec (Relevant plugins)

cloud_firestore: 0.13.3
cloud_functions: 0.4.1+5
google_sign_in: 4.1.4
firebase_auth: 0.15.2
firebase_storage: 3.0.11

Flutter doctor

[✓] Flutter (Channel stable, v1.12.13+hotfix.5, on Mac OS X 10.15.3 19D76, locale en-SG)

[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.0-rc2)
[✓] Xcode - develop for iOS and macOS (Xcode 11.3.1)
[✓] Android Studio (version 3.5)
[✓] IntelliJ IDEA Community Edition (version 2018.2.3)
[✓] VS Code (version 1.42.1)
[✓] Connected device (11 available)

Same here:

E/CloudFirestorePlugin(10092): java.lang.Exception: DoTransaction failed: UNAVAILABLE: Unable to resolve host firestore.googleapis.com E/CloudFirestorePlugin(10092): java.util.concurrent.ExecutionException: java.lang.Exception: DoTransaction failed: UNAVAILABLE: Unable to resolve host firestore.googleapis.com E/CloudFirestorePlugin(10092): at com.google.android.gms.tasks.Tasks.zzb(Unknown Source:61) E/CloudFirestorePlugin(10092): at com.google.android.gms.tasks.Tasks.await(Unknown Source:33) E/CloudFirestorePlugin(10092): at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$4.apply(CloudFirestorePlugin.java:527) E/CloudFirestorePlugin(10092): at io.flutter.plugins.firebase.cloudfirestore.CloudFirestorePlugin$4.apply(CloudFirestorePlugin.java:479) E/CloudFirestorePlugin(10092): at com.google.firebase.firestore.FirebaseFirestore.lambda$runTransaction$0(com.google.firebase:firebase-firestore@@21.3.0:301) E/CloudFirestorePlugin(10092): at com.google.firebase.firestore.FirebaseFirestore$$Lambda$5.call(Unknown Source:6)

@fenchai23 Transactions are supposed to fail when offline; as it appears that your device was.
But I assumed it hard crashed? It's not supposed to do that :)

hoc081098 added 2 commits September 4, 2019 12:30

Fix transaction crash on Android device

ee96f79

Update pub version

d13b8c4

hoc081098 requested review from collinjackson and kroikie as code owners September 4, 2019 05:39

collinjackson added 3 commits September 4, 2019 14:33

Bump for release

47c9b2f

Reformat

8a27331

Fix crash

928e052

collinjackson approved these changes Sep 4, 2019

View reviewed changes

creativecreatorormaybenot mentioned this pull request Sep 7, 2019

[cloud_firestore] Fix runTransaction Android crash #103

Closed

Merge remote-tracking branch 'origin/master' into fix/transaction_cra…

f02595a

…sh_android

Merge branch 'master' into fix/transaction_crash_android

2e7c183

googlebot added the cla: yes label Sep 23, 2019

collinjackson merged commit a24db9c into firebase:master Oct 8, 2019

collinjackson mentioned this pull request Oct 8, 2019

[cloud_firestore] Update pubspec.yaml #281

Merged

kroikie pushed a commit to collinjackson/flutterfire that referenced this pull request Nov 15, 2019

[cloud_firestore] Fixes crash on Android when run a transaction (fire…

7d1a96a

…base#87) * Fix transaction crash on Android device * Update pub version * Bump for release * Reformat * Fix crash

NickM-27 mentioned this pull request Dec 12, 2019

[firebase_messaging] MissingPluginException when building for macOS #1608

Closed

igorantolic mentioned this pull request Mar 11, 2020

[firebase_messaging] for iOS XCode Product/Archive failed with "Module 'firebase_messaging' not found" #2156

Closed

besidesordinary mentioned this pull request May 5, 2020

flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java uses unchecked or unsafe operations. Note: Recompile with -Xlint:unchecked for details. #2490

Closed

firebase locked and limited conversation to collaborators Aug 19, 2020

[cloud_firestore] Fixes crash on Android when run a transaction by hoc081098 · Pull Request #87 · firebase/flutterfire · GitHub

Uh oh!

[cloud_firestore] Fixes crash on Android when run a transaction - #87

[cloud_firestore] Fixes crash on Android when run a transaction#87collinjackson merged 7 commits intofirebase:masterfirebase/flutterfire:masterfrom hoc081098:fix/transaction_crash_androidhoc081098/flutterfire:fix/transaction_crash_androidCopy head branch name to clipboard

Conversation

hoc081098 commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Checklist

Breaking Change

Uh oh!

googlebot commented Sep 4, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

hoc081098 commented Sep 4, 2019

Uh oh!

googlebot commented Sep 4, 2019

Uh oh!

collinjackson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoc081098 commented Sep 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

creativecreatorormaybenot commented Sep 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DartMessenger.java line 35

io.flutter.embedding.engine.dart.DartMessenger$Reply.reply

Uh oh!

creativecreatorormaybenot commented Sep 13, 2019

Uh oh!

Tolik7 commented Sep 30, 2019

Uh oh!

hoc081098 commented Oct 1, 2019

Uh oh!

jorgercg commented Oct 4, 2019

Uh oh!

Tolik7 commented Oct 15, 2019

Uh oh!

bpaul7101 commented Mar 3, 2020

Caused by java.lang.AssertionError: INTERNAL ASSERTION FAILED: A transaction object cannot be used after its update callback has been invoked.

Uh oh!

fenchai23 commented Apr 19, 2020

Uh oh!

larssn commented Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

[cloud_firestore] Fixes crash on Android when run a transaction#87
collinjackson merged 7 commits into
firebase:masterfrom
hoc081098:fix/transaction_crash_android

hoc081098 commented Sep 4, 2019 •
edited
Loading

collinjackson left a comment •
edited
Loading

hoc081098 commented Sep 5, 2019 •
edited
Loading

creativecreatorormaybenot commented Sep 7, 2019 •
edited
Loading

larssn commented Jun 24, 2020 •
edited
Loading